happydave1 commented on code in PR #1138:
URL: https://github.com/apache/iceberg-go/pull/1138#discussion_r3394373122


##########
table/arrow_utils_test.go:
##########
@@ -430,6 +431,455 @@ func TestVariantArrowConversion(t *testing.T) {
        })
 }
 
+func assertGeoArrowWKB(t *testing.T, dt arrow.DataType, storage 
arrow.DataType, want geoarrow.Metadata) {
+       t.Helper()
+
+       wkb, ok := dt.(*geoarrow.WKBType)
+       require.True(t, ok, "expected *geoarrow.WKBType, got %T", dt)
+       assert.Equal(t, "geoarrow.wkb", wkb.ExtensionName())
+       assert.True(t, arrow.TypeEqual(storage, wkb.StorageType()))
+       assert.Equal(t, want.CRS, wkb.Metadata().CRS)
+       assert.Equal(t, want.Edges, wkb.Metadata().Edges)
+}
+
+func assertGeoArrowWKBMetadataJSON(t *testing.T, dt arrow.DataType, storage 
arrow.DataType, wantJSON string) {
+       t.Helper()
+
+       wkb, ok := dt.(*geoarrow.WKBType)
+       require.True(t, ok, "expected *geoarrow.WKBType, got %T", dt)
+       assert.Equal(t, "geoarrow.wkb", wkb.ExtensionName())
+       assert.True(t, arrow.TypeEqual(storage, wkb.StorageType()))
+
+       var want map[string]json.RawMessage
+       require.NoError(t, json.Unmarshal([]byte(wantJSON), &want))
+
+       meta := wkb.Metadata()
+
+       if wantCRS, ok := want["crs"]; ok {
+               assert.JSONEq(t, string(wantCRS), string(meta.CRS), "crs 
mismatch")
+       } else {
+               assert.Empty(t, meta.CRS, "expected omitted crs")
+       }
+
+       if wantEdges, ok := want["edges"]; ok {
+               var edges string
+               require.NoError(t, json.Unmarshal(wantEdges, &edges))
+               assert.Equal(t, geoarrow.EdgeInterpolation(edges), meta.Edges, 
"edges mismatch")
+       } else {
+               assert.Empty(t, meta.Edges, "expected omitted edges")
+       }
+}
+
+func jsonCRS(s string) json.RawMessage {
+       raw, _ := json.Marshal(s)
+
+       return raw
+}
+
+func TestIcebergGeoTypesToArrowSchema(t *testing.T) {
+       geomSRID, err := iceberg.GeometryTypeOf("srid:4326")
+       require.NoError(t, err)
+       geogKarney, err := iceberg.GeographyTypeOf("srid:4269", "karney")
+       require.NoError(t, err)
+
+       // Note that these tests below are based on arrow-rs tests 
(https://github.com/apache/arrow-rs/pull/10065)
+       geomSRID0, err := iceberg.GeometryTypeOf("srid:0")
+       require.NoError(t, err)
+       geomEPSG4267, err := iceberg.GeometryTypeOf("EPSG:4267")
+       require.NoError(t, err)
+       geogSpherical, err := iceberg.GeographyTypeOf("OGC:CRS84", "spherical")
+       require.NoError(t, err)
+       geogKarneyDefaultCRS, err := iceberg.GeographyTypeOf("OGC:CRS84", 
"karney")
+       require.NoError(t, err)
+       geogVincenty, err := iceberg.GeographyTypeOf("OGC:CRS84", "vincenty")
+       require.NoError(t, err)
+       geogAndoyer, err := iceberg.GeographyTypeOf("OGC:CRS84", "andoyer")
+       require.NoError(t, err)
+       geogThomas, err := iceberg.GeographyTypeOf("OGC:CRS84", "thomas")
+       require.NoError(t, err)
+       geogSRID0, err := iceberg.GeographyTypeOf("srid:0", "spherical")
+       require.NoError(t, err)
+       geogEPSG4267, err := iceberg.GeographyTypeOf("EPSG:4267", "spherical")
+       require.NoError(t, err)
+
+       defaultGeometry, err := iceberg.GeometryTypeOf("OGC:CRS84")
+       require.NoError(t, err)
+       geomPROJJSON3857, err := iceberg.GeometryTypeOf("EPSG:3857")
+       require.NoError(t, err)
+       geogEPSG4267Karney, err := iceberg.GeographyTypeOf("EPSG:4267", 
"karney")
+       require.NoError(t, err)
+       geogPROJJSON4267, err := iceberg.GeographyTypeOf("EPSG:4267", 
"spherical")
+       require.NoError(t, err)
+
+       typeCases := []struct {
+               name         string
+               ice          iceberg.Type
+               wantMetaJSON string
+       }{
+               // Geometry with default CRS (defaults to OGC:CRS84 per Parquet 
spec)
+               {
+                       name:         "geometry_default_crs",
+                       ice:          iceberg.GeometryType{},
+                       wantMetaJSON: `{"crs":"OGC:CRS84"}`,
+               },
+               // Geometry with srid:0 should result in an unset (omitted) CRS
+               {
+                       name:         "geometry_srid_0",
+                       ice:          geomSRID0,
+                       wantMetaJSON: `{}`,
+               },
+               // Geometry with custom CRSes (authority:code and partial 
projjson)
+               {
+                       name:         "geometry_epsg_4267",
+                       ice:          geomEPSG4267,
+                       wantMetaJSON: `{"crs":"EPSG:4267"}`,
+               },
+               // Geography with default CRS (default OGC:CRS84, spherical 
edges)
+               {
+                       name:         "geography_default_crs",
+                       ice:          iceberg.GeographyType{},
+                       wantMetaJSON: `{"crs":"OGC:CRS84","edges":"spherical"}`,
+               },
+               // Geography with explicit edges
+               {
+                       name:         "geography_explicit_spherical",
+                       ice:          geogSpherical,
+                       wantMetaJSON: `{"crs":"OGC:CRS84","edges":"spherical"}`,
+               },
+               {
+                       name:         "geography_karney",
+                       ice:          geogKarneyDefaultCRS,
+                       wantMetaJSON: `{"crs":"OGC:CRS84","edges":"karney"}`,
+               },
+               {
+                       name:         "geography_vincenty",
+                       ice:          geogVincenty,
+                       wantMetaJSON: `{"crs":"OGC:CRS84","edges":"vincenty"}`,
+               },
+               {
+                       name:         "geography_andoyer",
+                       ice:          geogAndoyer,
+                       wantMetaJSON: `{"crs":"OGC:CRS84","edges":"andoyer"}`,
+               },
+               {
+                       name:         "geography_thomas",
+                       ice:          geogThomas,
+                       wantMetaJSON: `{"crs":"OGC:CRS84","edges":"thomas"}`,
+               },
+               // Geography with srid:0 should result in an unset (omitted) 
CRS and spherical edges
+               {
+                       name:         "geography_srid_0",
+                       ice:          geogSRID0,
+                       wantMetaJSON: `{"edges":"spherical"}`,
+               },
+               // Geography with custom CRSes (authority:code and partial 
projjson)
+               {
+                       name:         "geography_epsg_4267",
+                       ice:          geogEPSG4267,
+                       wantMetaJSON: `{"crs":"EPSG:4267","edges":"spherical"}`,
+               },
+       }
+
+       // The following tests focus on edge cases and pinning specific 
behavior for read case
+       readOnlyCases := []struct {
+               name         string
+               ice          iceberg.Type
+               wantMetaJSON string
+       }{
+               {
+                       name:         "geometry_srid_0_with_crs_type",
+                       ice:          geomSRID0,
+                       wantMetaJSON: `{"crs_type":"authority_code"}`,
+               },
+               {
+                       name:         "case_insensitive_default_geometry",
+                       ice:          defaultGeometry,
+                       wantMetaJSON: `{"crs":"OgC:cRs84"}`,
+               },
+               {
+                       name:         
"case_insensitive_default_geometry_epsg_4326",
+                       ice:          defaultGeometry,
+                       wantMetaJSON: `{"crs":"EpSg:4326"}`,
+               },
+               {
+                       name:         "geometry_epsg_4326",
+                       ice:          defaultGeometry,
+                       wantMetaJSON: `{"crs":"epsg:4326"}`,
+               },
+
+               // Translated from arrow-rs geo logical type read tests 
(https://github.com/apache/arrow-rs/pull/10065)
+               // Geometry with no CRS should be GEOMETRY(srid:0)
+               {
+                       name:         "geometry_no_crs",
+                       ice:          geomSRID0,
+                       wantMetaJSON: `{}`,
+               },
+               // Geometry with string CRS
+               {
+                       name:         "geometry_epsg_4267_from_crs",
+                       ice:          geomEPSG4267,
+                       wantMetaJSON: `{"crs":"EPSG:4267"}`,
+               },
+               // Geometry with PROJJSON CRS
+               {
+                       name:         "geometry_projjson_epsg_3857",
+                       ice:          geomPROJJSON3857,
+                       wantMetaJSON: 
`{"crs":{"id":{"authority":"EPSG","code":3857}}}`,
+               },

Review Comment:
   Sorry, the naming is very bad here. These cases are the read cases so we 
start from `wantMetaJSON` and convert to logical Iceberg Type. I named the 
iceberg type `geomPROJJSON3857` because the `"crs"` field is a JSON object.
   
   I will change the naming of the read only tests to make this clearer. 



-- 
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]

Reply via email to