Uh oh!
There was an error while loading. Please reload this page.
Parquet: Compute geometry bounding box metrics - #17161
Conversation
ef2f67b to
4a1d8e8Compare
szehon-ho
left a comment
There was a problem hiding this comment.
Per-dimension NaN handling looks inconsistent with the spec's bounds rules for geometry.
The spec says null/NaN are skipped per coordinate dimension, and gives the example that POINT (1 NaN) contributes to X but not Y. A bbox is omitted only when a dimension has no valid values after aggregating across the whole file.
addXY currently skips the entire coordinate when either axis is NaN. That matches the single-geometry empty case, but not mixed files — e.g. POINT (1 NaN) + POINT (5 10) should yield bbox (1, 10)–(5, 10), while this implementation produces (5, 10)–(5, 10) (xmin too high). That can violate the manifest invariant that lower bounds must be ≤ all non-null, non-NaN values and lead to incorrect file pruning during scan planning.
Suggested fix: accumulate X and Y independently (update min/max only for non-NaN components), then emit a bbox only when both dimensions have at least one valid value. A test like POINT (1 NaN) + POINT (NaN 20) → (1, 20)–(1, 20) would lock this in.
| */ | ||
| public void addXY(double xCoord, double yCoord) { | ||
| if (Double.isNaN(xCoord) || Double.isNaN(yCoord)) { | ||
| return; |
There was a problem hiding this comment.
This skips the whole coordinate when either axis is NaN, but the spec skips per dimension. Consider accumulating X and Y independently so POINT (1 NaN) still contributes X=1 when other rows supply a valid Y.
f65a065 to
1756426Compare
Scan each geometry value's WKB coordinates with the core GeometryBoundsBuilder (apache#17509) and accumulate a 2D bounding box, written to lower_bounds/upper_bounds so geometry data files carry spatial bounds. The Parquet footer's lexicographic min/max over WKB bytes is not a spatial bound, so the box is produced on the writer-side value-scanning metrics channel -- the same path float and double use for the NaN counts footer statistics cannot provide. - GeometryFieldMetrics wraps GeometryBoundsBuilder and emits a FieldMetrics<GeospatialBound> whose lower/upper corners serialize through the existing geometry Conversions case; it carries no bounds when a dimension is absent or a value cannot be parsed. - The generic Parquet GeometryWriter folds each value into the box while writing byte-identical WKB. Geometry only, XY, planar. Geography bounds, average value size, and the Spark/ORC writers remain follow-ups.
1756426 to
ed14b97Compare| public void write(int repetitionLevel, ByteBuffer buffer) { | ||
| // Accumulate the bounding box before writing, so it reads the buffer's coordinates while the | ||
| // position is intact (the scanner reads a duplicate and leaves this buffer untouched). | ||
| metricsBuilder.addValue(buffer); |
There was a problem hiding this comment.
Please preserve the existing average WKB size metric while adding bounds. #17333 already made GeospatialWriter populate avg_value_size_in_bytes, but this writer replaces it with a builder that never records or returns the size, so generic Parquet geometry writes lose that metric. Could GeometryFieldMetrics.Builder accumulate the WKB size and carry the average into the returned FieldMetrics, and keep the existing TestMetrics assertion?
Computes file-level 2D bounding-box metrics for
geometrycolumns written toParquet. The bounds are stored in
lower_bounds/upper_bounds, making spatialfile pruning possible. This PR produces the bounds; expression and scan-planner
integration that consumes them is separate follow-up work.
This is the first slice of the geo bounds work (Phase 2), scoped to the clean,
unambiguous planar case.
Problem
The ordinary Parquet column min/max for WKB is a lexicographic byte bound, not a
spatial bound. Iceberg therefore diverts geometry/geography columns to
counts-only metrics in its current
ParquetMetricspath. Although newer Parquetmetadata can represent geospatial statistics separately, Iceberg does not
currently consume those footer bounds. As a result, geometry data files do not
carry spatial bounds in Iceberg metadata.
Approach
Compute the box while values are written, using the existing writer-side
value-scanning metrics channel -- the same path
floatanddoubleuse to trackNaN counts that ordinary footer statistics cannot provide
(
ParquetValueWriter.metrics()->ParquetWriter.metrics()):GeometryWriterwrites byte-identical WKB and folds eachvalue's XY coordinates into a running box.
GeometryWriterperforms the same accumulation after convertingSpark's typed
GeometryValto the pure WKB stored by Iceberg.FieldMetrics<GeospatialBound>whose lower and upper cornersserialize through the existing geometry
Conversionscase intolower_bounds/upper_bounds.the optional-field writer reconciles null counts, so the geometry builder sees
only non-null values.
This does not change
ParquetMetrics,ParquetWriter, orConversions.WKBBoundingBox(new inapi/geospatial, next toGeospatialBoundandBoundingBox) is a pure-Java WKB coordinate scanner with no JTS dependency. It:POINT,LINESTRING,POLYGON, multi-geometries,and collections);
spec -- for example,
POINT (1 NaN)contributes X=1, and another row may supplythe missing Y bound;
POINT EMPTYcontributes nothing; andnesting, and oversized counts fail with
IllegalArgumentExceptionrather thanreading out of bounds.
Scope
GEOMETRY only, 2D (XY), planar. This PR writes file bounds but does not add a
spatial predicate to the Expression API or wire spatial pruning into scan
planning.
Deliberately left as follow-ups:
numerical coverage guarantees, pole handling, and coordinate-range policy;
content_statsgeo_lower/geo_upperbridge;not produce spatial bounds);
MetricsConfigwill not retain bounds.Tests
TestWKBBoundingBoxcovers every geometry type; XY, Z, M, and ZM layouts;little-, big-, and mixed-endian inputs; nested and empty collections; empty
children; outer/interior rings; degenerate and differently oriented polygons;
per-axis NaN accumulation; infinities; malformed inputs; and nesting limits.
TestGeometryFieldMetricscovers cross-value aggregation, counts, and empty orno-value results.
TestMetrics.testMetricsForGeospatialTypesverifies that generic Parquet writesproduce the expected geometry bounds while geography remains bounds-less.
TestSparkParquetWriter.testGeospatialRoundTripverifies Spark 4.1 WKBround-trip, geometry bounds across multiple rows, null handling, and that
geography still produces no bounds.
AI Disclosure