huan233usc commented on code in PR #17509: URL: https://github.com/apache/iceberg/pull/17509#discussion_r3859847849
########## core/src/main/java/org/apache/iceberg/GeometryBoundsBuilder.java: ########## @@ -0,0 +1,431 @@ +/* + * 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 com.google.errorprone.annotations.FormatMethod; +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 X and Y bounds are produced. The spec also defines optional Z and M bounds, but producing + * them is a deliberate non-goal here: any Z and M ordinates in a value are read past rather than + * bounded, so an XYZ or XYZM value still parses and contributes its X and Y. + * + * <p>Every ring of a polygon, including interior rings, contributes to the bounds. In a valid + * polygon the holes lie inside the shell, so this is the shell's own box. + */ +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 DIMENSION_DIVISOR = 1000; + 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 ANY_DIMENSION = -1; + + private static final int MIN_RING_POINTS = 4; + + private final DimensionBounds xBounds = new DimensionBounds(); + private final DimensionBounds yBounds = new DimensionBounds(); + // set when a value could not be fully bounded -- malformed or unsupported WKB, or bytes left + // after the declared geometry -- so an object may be missing from the box; build() then + // suppresses the bounds + private boolean incomplete = false; + + /** + * 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>A value the parser cannot bound never fails the caller. Malformed WKB, a valid OGC type this + * builder does not support (such as PolyhedralSurface, TIN, or Triangle), and bytes left after + * the declared geometry all mean an object may be missing from the box, so {@link #build()} + * returns no bounds for the file instead. Suppressing optional metrics is safe where failing the + * write is not, since the write cannot skip the value or retry past it. + * + * @param wkb a buffer containing one WKB geometry + * @throws IllegalArgumentException if {@code wkb} is null + */ + public void addValue(ByteBuffer wkb) { Review Comment: Done — `addValue` returns early once `incomplete` is set, so the rest of the file skips the walk. ########## core/src/main/java/org/apache/iceberg/GeometryBoundsBuilder.java: ########## @@ -0,0 +1,431 @@ +/* + * 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 com.google.errorprone.annotations.FormatMethod; +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 X and Y bounds are produced. The spec also defines optional Z and M bounds, but producing + * them is a deliberate non-goal here: any Z and M ordinates in a value are read past rather than + * bounded, so an XYZ or XYZM value still parses and contributes its X and Y. + * + * <p>Every ring of a polygon, including interior rings, contributes to the bounds. In a valid + * polygon the holes lie inside the shell, so this is the shell's own box. + */ +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 DIMENSION_DIVISOR = 1000; + 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 ANY_DIMENSION = -1; + + private static final int MIN_RING_POINTS = 4; + + private final DimensionBounds xBounds = new DimensionBounds(); + private final DimensionBounds yBounds = new DimensionBounds(); + // set when a value could not be fully bounded -- malformed or unsupported WKB, or bytes left + // after the declared geometry -- so an object may be missing from the box; build() then + // suppresses the bounds + private boolean incomplete = false; + + /** + * 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>A value the parser cannot bound never fails the caller. Malformed WKB, a valid OGC type this + * builder does not support (such as PolyhedralSurface, TIN, or Triangle), and bytes left after + * the declared geometry all mean an object may be missing from the box, so {@link #build()} + * returns no bounds for the file instead. Suppressing optional metrics is safe where failing the + * write is not, since the write cannot skip the value or retry past it. + * + * @param wkb a buffer containing one WKB geometry + * @throws IllegalArgumentException if {@code wkb} is null + */ + public void addValue(ByteBuffer wkb) { + Preconditions.checkArgument(wkb != null, "Invalid WKB buffer: null"); + ByteBuffer buffer = wkb.duplicate(); + try { + parseGeometry(buffer, ANY_GEOMETRY, ANY_DIMENSION); + } catch (InvalidWkbException e) { + incomplete = true; + return; + } + + // a complete geometry that leaves bytes behind was not fully parsed (for example a multi + // geometry whose declared element count is short), so an object never reached the bounds + if (buffer.hasRemaining()) { + incomplete = true; + } + } + + /** + * Builds the bounding box covering every geometry added, or {@code null} if either the X or Y + * dimension has no value, or if any value could not be bounded (see {@link #addValue}). + */ + public BoundingBox build() { + if (incomplete || !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 expectedType, int expectedDimension) { + // a geometry header is a one-byte order flag followed by a four-byte type code: + // +-------+-----------------------+ + // | order | type code | + // | (1 B) | (4 B) | + // +-------+-----------------------+ + checkRemaining(buffer, Byte.BYTES + Integer.BYTES); + + // each geometry sets its own byte order; restore the caller's order before returning so a Review Comment: Dropped the save/restore and reworded the comment, since the order set is never read across a sibling. ########## core/src/main/java/org/apache/iceberg/GeometryBoundsBuilder.java: ########## @@ -0,0 +1,431 @@ +/* + * 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 com.google.errorprone.annotations.FormatMethod; +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 X and Y bounds are produced. The spec also defines optional Z and M bounds, but producing + * them is a deliberate non-goal here: any Z and M ordinates in a value are read past rather than + * bounded, so an XYZ or XYZM value still parses and contributes its X and Y. + * + * <p>Every ring of a polygon, including interior rings, contributes to the bounds. In a valid + * polygon the holes lie inside the shell, so this is the shell's own box. + */ +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 DIMENSION_DIVISOR = 1000; + 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 ANY_DIMENSION = -1; + + private static final int MIN_RING_POINTS = 4; + + private final DimensionBounds xBounds = new DimensionBounds(); + private final DimensionBounds yBounds = new DimensionBounds(); + // set when a value could not be fully bounded -- malformed or unsupported WKB, or bytes left + // after the declared geometry -- so an object may be missing from the box; build() then + // suppresses the bounds + private boolean incomplete = false; + + /** + * 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>A value the parser cannot bound never fails the caller. Malformed WKB, a valid OGC type this + * builder does not support (such as PolyhedralSurface, TIN, or Triangle), and bytes left after + * the declared geometry all mean an object may be missing from the box, so {@link #build()} + * returns no bounds for the file instead. Suppressing optional metrics is safe where failing the + * write is not, since the write cannot skip the value or retry past it. + * + * @param wkb a buffer containing one WKB geometry + * @throws IllegalArgumentException if {@code wkb} is null + */ + public void addValue(ByteBuffer wkb) { + Preconditions.checkArgument(wkb != null, "Invalid WKB buffer: null"); + ByteBuffer buffer = wkb.duplicate(); + try { + parseGeometry(buffer, ANY_GEOMETRY, ANY_DIMENSION); + } catch (InvalidWkbException e) { + incomplete = true; + return; + } + + // a complete geometry that leaves bytes behind was not fully parsed (for example a multi + // geometry whose declared element count is short), so an object never reached the bounds + if (buffer.hasRemaining()) { + incomplete = true; + } + } + + /** + * Builds the bounding box covering every geometry added, or {@code null} if either the X or Y + * dimension has no value, or if any value could not be bounded (see {@link #addValue}). + */ + public BoundingBox build() { + if (incomplete || !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 expectedType, int expectedDimension) { + // a geometry header is a one-byte order flag followed by a four-byte type code: + // +-------+-----------------------+ + // | order | type code | + // | (1 B) | (4 B) | + // +-------+-----------------------+ + checkRemaining(buffer, Byte.BYTES + Integer.BYTES); + + // 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 InvalidWkbException("Invalid WKB byte order: " + order); + } + + parseGeometryBodyAndUpdateBound(buffer, expectedType, expectedDimension); + buffer.order(callerOrder); + } + + private void parseGeometryBodyAndUpdateBound( + ByteBuffer buffer, int expectedType, int expectedDimension) { + long typeCode = Integer.toUnsignedLong(buffer.getInt()); + int dimensionGroup = (int) (typeCode / DIMENSION_DIVISOR); + int geometryType = (int) (typeCode % DIMENSION_DIVISOR); + // only the seven OGC types in XY/XYZ/XYM/XYZM are bounded here; other valid OGC types (such as + // PolyhedralSurface, TIN, and Triangle) are unsupported and cost the value its bounds + checkWkb( + geometryType >= TYPE_POINT + && geometryType <= TYPE_GEOMETRY_COLLECTION + && dimensionGroup <= XYZM_GROUP, + "Invalid or unsupported WKB geometry type: %s", + typeCode); + // an element of a multi geometry or collection must match its parent's member type and + // dimensions; if/throw so the message is built only when a value is actually rejected + if (expectedType != ANY_GEOMETRY && geometryType != expectedType) { + throw new InvalidWkbException( + "Invalid WKB: expected geometry type " + + typeName(expectedType) + + " but found " + + typeName(geometryType)); + } + if (expectedDimension != ANY_DIMENSION && dimensionGroup != expectedDimension) { + throw new InvalidWkbException( + "Invalid WKB: expected dimensions " + + dimensionName(expectedDimension) + + " but found " + + dimensionName(dimensionGroup)); + } + + int numDimensions = numDimensions(dimensionGroup); + + switch (geometryType) { Review Comment: Converted the dispatch, `numDimensions`, `typeName`, and `dimensionName` to `case X ->` and dropped the unreachable default. -- 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]
