szehon-ho commented on code in PR #17509: URL: https://github.com/apache/iceberg/pull/17509#discussion_r3807759942
########## core/src/main/java/org/apache/iceberg/GeometryBoundsBuilder.java: ########## @@ -0,0 +1,305 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import org.apache.iceberg.geospatial.BoundingBox; +import org.apache.iceberg.geospatial.GeospatialBound; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; + +/** + * Accumulates geometry bounds from values encoded as Well-Known Binary (WKB). + * + * <p>The seven OGC geometry types are supported: point, line string, polygon, multi point, multi + * line string, multi polygon, and geometry collection. + * + * <p>Coordinates are tracked independently for the X and Y dimensions. A {@code NaN} ordinate marks + * an empty value and does not contribute to its dimension; an infinite ordinate is a real position + * and is kept as a bound, since the spec forbids only NaN as a lower or upper bound. No bounds are + * produced unless both dimensions are present. + * + * <p>These bounds apply to {@code geometry} columns, whose edges are always interpolated linearly, + * so a box that contains every vertex contains the whole geometry. They are not valid for {@code + * geography} columns: geodesic edges can reach beyond their endpoints, longitude is periodic, and a + * geography box may cross the antimeridian. + * + * <p>Only the X and Y dimensions contribute to the box. Z and M ordinates are valid in the ISO WKB + * serializations that Iceberg accepts, so they are read past and ignored rather than rejected. + * + * <p>The bounds of a polygon are derived from its exterior ring alone, which assumes OGC-valid + * polygons whose interior rings lie within the shell. This matches the envelope computed for a + * polygon by geometry libraries such as JTS. Iceberg does not validate geometries, so a polygon + * with a hole extending past its shell produces bounds that do not contain the geometry. + */ +class GeometryBoundsBuilder { + + private static final int TYPE_POINT = 1; + private static final int TYPE_LINE_STRING = 2; + private static final int TYPE_POLYGON = 3; + private static final int TYPE_MULTI_POINT = 4; + private static final int TYPE_MULTI_LINE_STRING = 5; + private static final int TYPE_MULTI_POLYGON = 6; + private static final int TYPE_GEOMETRY_COLLECTION = 7; + private static final int ANY_GEOMETRY = 0; + + // ISO WKB encodes the dimensions of a geometry in the thousands digit of its type code + private static final int XY_GROUP = 0; + private static final int XYZ_GROUP = 1; + private static final int XYM_GROUP = 2; + private static final int XYZM_GROUP = 3; + + private static final int MAX_DEPTH = 100; + + private final DimensionBounds xBounds = new DimensionBounds(); + private final DimensionBounds yBounds = new DimensionBounds(); + + /** + * Adds one WKB geometry value to these bounds. + * + * <p>The input is read through a duplicate, so its position and limit are left unchanged. + * + * <p>If this throws, the builder's state is undefined: coordinates parsed before the failure may + * already be folded in. A caller that continues after a rejected value must discard this builder. + * + * @param wkb a buffer containing exactly one WKB geometry + * @throws IllegalArgumentException if the WKB is malformed + */ + public void addValue(ByteBuffer wkb) { + Preconditions.checkArgument(wkb != null, "Invalid WKB buffer: null"); + ByteBuffer buffer = wkb.duplicate(); + parseGeometry(buffer, 0, ANY_GEOMETRY); + Preconditions.checkArgument(!buffer.hasRemaining(), "Invalid WKB: trailing data"); + } + + /** + * Builds the bounding box covering every geometry added, or {@code null} if either the X or Y + * dimension has no value. + */ + public BoundingBox build() { + if (!xBounds.hasValue() || !yBounds.hasValue()) { + return null; + } + + GeospatialBound min = GeospatialBound.createXY(xBounds.lower(), yBounds.lower()); + GeospatialBound max = GeospatialBound.createXY(xBounds.upper(), yBounds.upper()); + return new BoundingBox(min, max); + } + + private void parseGeometry(ByteBuffer buffer, int depth, int expectedType) { + Preconditions.checkArgument(depth <= MAX_DEPTH, "Invalid WKB: nesting too deep"); + checkRemaining(buffer, 5); + + // each geometry sets its own byte order; restore the caller's order before returning so a + // sibling read after a nested geometry is not misread with the wrong endianness + ByteOrder callerOrder = buffer.order(); + byte order = buffer.get(); + if (order == 0) { + buffer.order(ByteOrder.BIG_ENDIAN); + } else if (order == 1) { + buffer.order(ByteOrder.LITTLE_ENDIAN); + } else { + throw new IllegalArgumentException("Invalid WKB byte order: " + order); + } + + try { + parseGeometryBody(buffer, depth, expectedType); + } finally { + buffer.order(callerOrder); + } + } + + private void parseGeometryBody(ByteBuffer buffer, int depth, int expectedType) { + long typeCode = buffer.getInt() & 0xFFFFFFFFL; + long dimensionGroup = typeCode / 1000; + int geometryType = (int) (typeCode % 1000); + Preconditions.checkArgument( + geometryType >= TYPE_POINT + && geometryType <= TYPE_GEOMETRY_COLLECTION + && dimensionGroup <= XYZM_GROUP, + "Invalid or unsupported WKB geometry type: %s", + typeCode); + Preconditions.checkArgument( + expectedType == ANY_GEOMETRY || geometryType == expectedType, + "Invalid WKB: expected geometry type %s but found %s", + typeName(expectedType), + typeName(geometryType)); + + int numDimensions = numDimensions(dimensionGroup); + + switch (geometryType) { + case TYPE_POINT: + readCoordinate(buffer, numDimensions); + break; + case TYPE_LINE_STRING: + readCoordinateSequence(buffer, numDimensions, true); + break; + case TYPE_POLYGON: + readPolygon(buffer, numDimensions); + break; + case TYPE_MULTI_POINT: + readCollection(buffer, depth, TYPE_POINT); + break; + case TYPE_MULTI_LINE_STRING: + readCollection(buffer, depth, TYPE_LINE_STRING); + break; + case TYPE_MULTI_POLYGON: + readCollection(buffer, depth, TYPE_POLYGON); + break; + case TYPE_GEOMETRY_COLLECTION: + readCollection(buffer, depth, ANY_GEOMETRY); + break; + default: + throw new IllegalArgumentException("Invalid or unsupported WKB geometry type: " + typeCode); + } + } + + private static String typeName(int geometryType) { + switch (geometryType) { + case TYPE_POINT: + return "Point"; + case TYPE_LINE_STRING: + return "LineString"; + case TYPE_POLYGON: + return "Polygon"; + case TYPE_MULTI_POINT: + return "MultiPoint"; + case TYPE_MULTI_LINE_STRING: + return "MultiLineString"; + case TYPE_MULTI_POLYGON: + return "MultiPolygon"; + case TYPE_GEOMETRY_COLLECTION: + return "GeometryCollection"; + default: + return String.valueOf(geometryType); + } + } + + private static int numDimensions(long dimensionGroup) { + switch ((int) dimensionGroup) { + case XY_GROUP: + return 2; + case XYZ_GROUP: + case XYM_GROUP: + return 3; + default: // XYZM_GROUP, the only remaining group the caller accepts + return 4; + } + } + + private void readPolygon(ByteBuffer buffer, int numDimensions) { Review Comment: Suggest letting interior rings contribute too. This is the only caller that passes `updateBounds = false`, so `readCoordinateSequence` could then drop that `boolean` parameter and its skip branch entirely. A hole extending past the shell under-covers, violating `format/spec.md:768`, so the file is pruned from a query it should match: missing rows, no error. Counterpoint: JTS and Parquet's footer stats use the shell alone, so bounds would diverge from them. -- 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]
