tanmayrauth commented on code in PR #1371:
URL: https://github.com/apache/iceberg-go/pull/1371#discussion_r3540115284
##########
table/internal/parquet_files_test.go:
##########
@@ -1348,3 +1349,87 @@ func TestShreddedVariantReadRoundTrip(t *testing.T) {
assert.EqualValues(t, 2, obj.NumElements(), "row %d should have
a + city", i)
}
}
+
+// TestWriteDataFileGeoBounds writes geometry and geography columns through the
+// full ParquetFileWriter path and asserts that the resulting DataFile carries
+// geometry single-point bounds (Iceberg geospatial single-value serialization)
+// in the manifest entry, while geography bounds are omitted as unsafe.
+func TestWriteDataFileGeoBounds(t *testing.T) {
Review Comment:
Added a comment stating the test guards the createStatsAgg panic via the
isGeoType gate (geo types have no generic stats aggregator), and split the
full-mode path into its own named sub-case so the intent reads clearly. The
MetricModeNone/Counts sub-cases above cover the applyGeoBounds gating.
##########
literals.go:
##########
@@ -174,6 +174,22 @@ func LiteralFromBytes(typ Type, data []byte) (Literal,
error) {
var v BinaryLiteral
err := v.UnmarshalBinary(data)
+ return v, err
+ case GeometryType, GeographyType:
+ // Geometry/Geography single-value bounds use the Iceberg
geospatial
+ // serialization (spec Appendix D): little-endian float64
coordinates in
+ // X, Y[, Z][, M] order, i.e. 16, 24, or 32 bytes — not WKB.
iceberg-go
+ // has no geo literal for predicate evaluation, so the validated
+ // coordinate bytes are returned as an opaque BinaryLiteral.
+ switch len(data) {
+ case 16, 24, 32:
Review Comment:
Both addressed: annotated case 16, 24, 32: as an intentional valid-sizes
fall-through, and added a warning that the returned BinaryLiteral holds raw
little-endian coordinates — a future geo predicate evaluator must check the
original type rather than treating it as a comparable binary bound.
##########
table/internal/parquet_files.go:
##########
@@ -343,6 +344,34 @@ type ParquetFileWriter struct {
info WriteFileInfo
partition map[int]any
colMapping map[string]int
+ geoCols []geoColumn
+ geoAccs map[int]*geoBoundsAccumulator
+}
+
+// geoColumn locates a top-level geometry/geography column: its position in the
+// Arrow record batch and the Iceberg field ID its bounds are recorded under.
+type geoColumn struct {
+ colIdx int
+ fieldID int
+}
+
+// collectGeoColumns finds top-level WKB-encoded geo columns in the Arrow
schema
+// and pairs each with its Iceberg field ID. Geo bounds for columns nested
inside
+// structs/lists/maps are not yet computed.
+func collectGeoColumns(sc *arrow.Schema, colMapping map[string]int)
[]geoColumn {
+ var result []geoColumn
+ for i, f := range sc.Fields() {
Review Comment:
Added // TODO(#992): compute geo bounds for geo columns nested in
structs/lists/maps and noted the divergence from Java/PyIceberg. I anchored it
to #992 (this work's parent) for now — happy to file a dedicated follow-up
issue and repoint the TODO if you'd prefer it tracked separately.
--
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]
