davidchaava commented on code in PR #28740:
URL: https://github.com/apache/flink/pull/28740#discussion_r3803551758
##########
docs/static/generated/rest_v4_sql_gateway.yml:
##########
@@ -516,6 +516,7 @@ components:
- DESCRIPTOR
- VARIANT
- BITMAP
+ - GEOGRAPHY
Review Comment:
Yes, agreed. Since the generated REST schema advertises `GEOGRAPHY`, the SQL
Gateway should support the logical type in the same PR.
We added `GEOGRAPHY` cases to `LogicalTypeJsonSerializer` and
`LogicalTypeJsonDeserializer`, together with JSON round-trip test coverage.
This allows REST clients to serialize and deserialize `GEOGRAPHY` type metadata
consistently.
The SQL constructors, accessors, and spatial functions are implemented
separately in the follow-up PR
[#28788](https://github.com/apache/flink/pull/28788).
##########
docs/content/docs/sql/reference/data-types.md:
##########
@@ -219,6 +219,7 @@ The default planner supports the following set of SQL types:
| Structured types | Only exposed in user-defined functions yet. |
| `VARIANT` | |
| `BITMAP` | |
+| `GEOGRAPHY` | Geography values in OGC:CRS84. |
Review Comment:
Addressed. I expanded the GEOGRAPHY documentation in
`docs/content/docs/sql/reference/data-types.md`.
It now explains CRS, SRID, ISO WKB, and EWKB, and includes references for
OGC:CRS84, OGC Simple Feature Access, and EWKB. I also clarified that the v1
contract uses standard 2D ISO WKB, while EWKB is not supported and CRS/SRID
metadata is not stored in the WKB payload.
Thanks for pointing this out.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/api/DataTypes.java:
##########
@@ -1075,6 +1076,15 @@ public static DataType BITMAP() {
return new AtomicDataType(new BitmapType());
}
+ /**
+ * Data type of geography data.
+ *
+ * @see GeographyType
+ */
+ public static DataType GEOGRAPHY() {
Review Comment:
Yes, we did need a dedicated converter for `GEOGRAPHY`, so we added one
following the existing identity-converter pattern.
`DataFormatConverters` now registers `GeographyConverter` for both
`GeographyData` and `BinaryGeographyData`. The converter preserves values as-is
and reads row fields through `RowData.getGeography()`.
We also added coverage for converter lookup, nullable and non-nullable
types, WKB round-trip, row extraction, null handling, `BinaryGeographyData`
bridging, and nested `ARRAY<GEOGRAPHY>` conversion.
Thanks for pointing this out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
