paleolimbot commented on code in PR #1138:
URL: https://github.com/apache/iceberg-go/pull/1138#discussion_r3383193433
##########
table/arrow_utils.go:
##########
@@ -1828,3 +1844,72 @@ func positionDeleteRecordsToDataFilesDV(ctx
context.Context, rootLocation string
}
}
}
+
+func geoArrowCRSToIcebergCRS(meta geoarrow.Metadata) (string, error) {
+ if len(meta.CRS) == 0 && meta.CRSType == "" {
+ return "srid:0", nil
+ }
+
+ var crs string
+ if len(meta.CRS) > 0 {
+ if err := json.Unmarshal(meta.CRS, &crs); err != nil {
+ return "", fmt.Errorf("invalid geoarrow CRS metadata:
%w", err)
+ }
+ }
+
+ if crs == "OGC:CRS84" || crs == "EPSG:4326" {
Review Comment:
The ideal behaviour is that all of these become the Iceberg default CRS:
- `"crs": "EPSG:4326"` and `"crs": "OGC:CRS84"` (case insensitive)
- `"crs": {..., "id":{"authority": "OGC", "code": "CRS84"}`
- `"crs": {..., "id":{"authority": "EPSG", "code": 4326}`
- - `"crs": {..., "id":{"authority": "EPSG", "code": "4326"}`
When reading an iceberg default CRS to GeoArrow, emit `"crs": "OGC:CRS84"`.
I think your PR is currently missing this case.
This asymmetry is helpful to improve the compatibility of iceberg tables
(e.g., for readers that can't or don't want to understand CRSes and only handle
the default case).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
