[SPARK-55530][Geo][SQL] Support Geo result sets in Hive and Thrift server#54325
[SPARK-55530][Geo][SQL] Support Geo result sets in Hive and Thrift server#54325uros-db wants to merge 7 commits intoapache:masterfrom
Conversation
9bb3128 to
73e7ab9
Compare
uros-db
left a comment
There was a problem hiding this comment.
@cloud-fan Please review.
| case (g: Geometry, dt: GeometryType) => | ||
| val internalGeom = STUtils.serializeGeomFromWKB(g, dt) | ||
| val s = STUtils.stAsEwkt(internalGeom).toString | ||
| if (nested) "\"" + s + "\"" else s |
There was a problem hiding this comment.
not related to this PR, but seems we forget to do the same for VariantType
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/RowSetUtils.scala
Show resolved
Hide resolved
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/RowSetUtils.scala
Outdated
Show resolved
Hide resolved
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/RowSetUtils.scala
Outdated
Show resolved
Hide resolved
…/thriftserver/RowSetUtils.scala Co-authored-by: Wenchen Fan <cloud0fan@gmail.com>
|
CI failure looks unrelated. Otherwise, this should be ready @cloud-fan. |
szehon-ho
left a comment
There was a problem hiding this comment.
mostly lgtm, small suggestions
| WkbReader reader = new WkbReader(true); | ||
| GeometryModel model = reader.read(Arrays.copyOfRange( | ||
| getBytes(), WKB_OFFSET, getBytes().length)); | ||
| return model.toString(); |
There was a problem hiding this comment.
i think it would have been cleaner to have explicit toWKT in GeometryModel, as toString may change. It can be separate from this pr though
| public byte[] toEwkt() { | ||
| // Once EWKT conversion is implemented, it should support various precisions. | ||
| throw new UnsupportedOperationException("Geography EWKT conversion is not yet supported."); | ||
| String ewkt = "SRID=" + srid() + ";" + toWktInternal(); |
There was a problem hiding this comment.
ok, so all wkt is valid ewkt, and we add extra SRID=... make sense to me.
| public byte[] toWkt() { | ||
| // Once WKT conversion is implemented, it should support various precisions. | ||
| throw new UnsupportedOperationException("Geometry WKT conversion is not yet supported."); | ||
| return toWktInternal().getBytes(StandardCharsets.UTF_8); |
There was a problem hiding this comment.
we could make this in Geo base class, but its quite small
| } | ||
| } | ||
| case gt: GeometryType => | ||
| val possibleGeometriesWKB = Seq( |
There was a problem hiding this comment.
can you comment what these values are?
What changes were proposed in this pull request?
Implement geospatial data output display in Extended Well-Known Text (EWKT) format and add the appropriate geospatial type mapping in Hive Thrift Server.
Why are the changes needed?
Display Geometry and Geography values in a human-readable format, which allows projecting geospatial data in Spark SQL and Thrift Server.
Does this PR introduce any user-facing change?
Yes, geospatial data is now displayed using EWKT.
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
No.