huan233usc commented on code in PR #17509: URL: https://github.com/apache/iceberg/pull/17509#discussion_r3731703489
########## api/src/main/java/org/apache/iceberg/geospatial/GeometryBoundsCollector.java: ########## @@ -0,0 +1,230 @@ +/* + * 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.geospatial; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +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. WKB values carrying Z or M dimensions are + * rejected. + * + * <p>Coordinates are tracked independently for the X and Y dimensions. {@code NaN} values do not + * contribute to a dimension, and no bounds are produced unless both dimensions are present. + */ +public final class GeometryBoundsCollector { + + 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; + + private static final int MAX_DEPTH = 100; + + private final DimensionBounds xBounds = new DimensionBounds(); + private final DimensionBounds yBounds = new DimensionBounds(); + + // reusable copies of the accumulated bounds, used to undo a partially parsed value + private final DimensionBounds xSaved = new DimensionBounds(); + private final DimensionBounds ySaved = new DimensionBounds(); + + /** + * Adds the coordinates from one WKB geometry to these bounds. + * + * <p>The input is read through a duplicate, so its position and limit are left unchanged. + * + * @param wkb a buffer containing exactly one WKB geometry + * @throws IllegalArgumentException if the WKB is invalid or unsupported + */ + public void add(ByteBuffer wkb) { + Preconditions.checkArgument(wkb != null, "Invalid WKB buffer: null"); + xSaved.copyFrom(xBounds); + ySaved.copyFrom(yBounds); + ByteBuffer buffer = wkb.duplicate(); + try { + parseGeometry(buffer, 0, ANY_GEOMETRY); + Preconditions.checkArgument(!buffer.hasRemaining(), "Invalid WKB: trailing data"); + } catch (RuntimeException e) { + xBounds.copyFrom(xSaved); + yBounds.copyFrom(ySaved); + throw e; + } + } + + /** Returns the accumulated bounding box, or {@code null} if either X or Y has no value. */ + public BoundingBox boundingBox() { + 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); + + 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); + } + + long typeCode = buffer.getInt() & 0xFFFFFFFFL; + int geometryType = (int) (typeCode % 1000); + Preconditions.checkArgument( + typeCode / 1000 == 0, "Unsupported WKB: only 2D geometries are supported"); Review Comment: Took the first option — `numDimensions()` now derives the ordinate count from the type code's dimension group and `readCoordinate` skips past Z and M, so XYZ/XYM/XYZM values contribute their XY extent instead of failing or discarding the box. Thanks for pointing at #17161: that handling was lost when this PR was split out, not deliberately dropped. The `aborted` state the previous revision used is gone with it, which also restores the strict trailing-data check it had to relax. ########## api/src/main/java/org/apache/iceberg/geospatial/GeometryBoundsCollector.java: ########## @@ -0,0 +1,230 @@ +/* + * 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.geospatial; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +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. WKB values carrying Z or M dimensions are + * rejected. + * + * <p>Coordinates are tracked independently for the X and Y dimensions. {@code NaN} values do not + * contribute to a dimension, and no bounds are produced unless both dimensions are present. + */ +public final class GeometryBoundsCollector { + + 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; + + private static final int MAX_DEPTH = 100; + + private final DimensionBounds xBounds = new DimensionBounds(); + private final DimensionBounds yBounds = new DimensionBounds(); + + // reusable copies of the accumulated bounds, used to undo a partially parsed value + private final DimensionBounds xSaved = new DimensionBounds(); + private final DimensionBounds ySaved = new DimensionBounds(); + + /** + * Adds the coordinates from one WKB geometry to these bounds. + * + * <p>The input is read through a duplicate, so its position and limit are left unchanged. + * + * @param wkb a buffer containing exactly one WKB geometry + * @throws IllegalArgumentException if the WKB is invalid or unsupported + */ + public void add(ByteBuffer wkb) { + Preconditions.checkArgument(wkb != null, "Invalid WKB buffer: null"); + xSaved.copyFrom(xBounds); + ySaved.copyFrom(yBounds); + ByteBuffer buffer = wkb.duplicate(); + try { + parseGeometry(buffer, 0, ANY_GEOMETRY); + Preconditions.checkArgument(!buffer.hasRemaining(), "Invalid WKB: trailing data"); + } catch (RuntimeException e) { + xBounds.copyFrom(xSaved); + yBounds.copyFrom(ySaved); + throw e; + } + } + + /** Returns the accumulated bounding box, or {@code null} if either X or Y has no value. */ + public BoundingBox boundingBox() { + 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); + + 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); + } + + long typeCode = buffer.getInt() & 0xFFFFFFFFL; + int geometryType = (int) (typeCode % 1000); + Preconditions.checkArgument( + typeCode / 1000 == 0, "Unsupported WKB: only 2D geometries are supported"); + Preconditions.checkArgument( + expectedType == ANY_GEOMETRY || geometryType == expectedType, + "Invalid WKB: expected geometry type %s but found %s", + expectedType, + geometryType); + + switch (geometryType) { + case TYPE_POINT: + readCoordinate(buffer); + break; + case TYPE_LINE_STRING: + readCoordinateSequence(buffer, true); + break; + case TYPE_POLYGON: + readPolygon(buffer); + 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 void readPolygon(ByteBuffer buffer) { + int numRings = readCount(buffer); + if (numRings > 0) { + readCoordinateSequence(buffer, true); + } + + // interior rings are contained by the exterior ring and cannot widen the bounds + for (int i = 1; i < numRings; i += 1) { + readCoordinateSequence(buffer, false); Review Comment: Promoted to the class javadoc. ########## api/src/main/java/org/apache/iceberg/geospatial/GeometryBoundsCollector.java: ########## @@ -0,0 +1,230 @@ +/* + * 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.geospatial; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +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. WKB values carrying Z or M dimensions are + * rejected. + * + * <p>Coordinates are tracked independently for the X and Y dimensions. {@code NaN} values do not + * contribute to a dimension, and no bounds are produced unless both dimensions are present. + */ +public final class GeometryBoundsCollector { Review Comment: Neither needs a WKB walk in `api`. #14101's `BoundingBoxLiteral` deserializes through `BoundingBox.fromByteBuffer`, which reads the serialized box form — two fixed-width corner points — not a WKB geometry; every other construction site in that PR passes two `GeospatialBound`s directly. #17175 compares a constant box against bounds already read from manifests and touches `BoundingBox.java` by a single line. So the parser is now package-private in `core`, next to where `GeometryFieldMetrics` will live, and stays off the tracked surface. ########## api/src/main/java/org/apache/iceberg/geospatial/GeometryBoundsCollector.java: ########## @@ -0,0 +1,230 @@ +/* + * 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.geospatial; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +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. WKB values carrying Z or M dimensions are + * rejected. + * + * <p>Coordinates are tracked independently for the X and Y dimensions. {@code NaN} values do not + * contribute to a dimension, and no bounds are produced unless both dimensions are present. + */ +public final class GeometryBoundsCollector { + + 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; + + private static final int MAX_DEPTH = 100; + + private final DimensionBounds xBounds = new DimensionBounds(); + private final DimensionBounds yBounds = new DimensionBounds(); + + // reusable copies of the accumulated bounds, used to undo a partially parsed value + private final DimensionBounds xSaved = new DimensionBounds(); + private final DimensionBounds ySaved = new DimensionBounds(); Review Comment: Dropped. With Z/M no longer aborting, the only remaining failures are malformed input, which the planned caller lets propagate. ########## core/src/test/java/org/apache/iceberg/TestGeometryBoundsCollector.java: ########## @@ -0,0 +1,196 @@ +/* + * 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.geospatial; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +public class TestGeometryBoundsCollector { Review Comment: Prefixes were already dropped; the class and its methods are package-private now too. ########## api/src/test/java/org/apache/iceberg/geospatial/TestGeometryBoundsCollector.java: ########## @@ -0,0 +1,196 @@ +/* + * 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.geospatial; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +public class TestGeometryBoundsCollector { + + @ParameterizedTest(name = "{0}") + @MethodSource("boundingBoxCases") + public void testBoundingBox(String wkt, String hexWkb, BoundingBox expected) { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + ByteBuffer wkb = decode(hexWkb); + int position = wkb.position(); + int limit = wkb.limit(); + + bounds.add(wkb); + + assertThat(wkb.position()).as(wkt).isEqualTo(position); + assertThat(wkb.limit()).as(wkt).isEqualTo(limit); + assertThat(bounds.boundingBox()).as(wkt).isEqualTo(expected); + } + + @Test + public void testNoBoundsWhenOneDimensionIsMissing() { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + bounds.add(decode("0101000000000000000000f03f000000000000f87f")); + + assertThat(bounds.boundingBox()).as("POINT(1 NaN)").isNull(); + } + + @Test + public void testBoundsAcrossValuesWithMissingCoordinates() { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + bounds.add(decode("0101000000000000000000f03f000000000000f87f")); + bounds.add(decode("0101000000000000000000f87f0000000000000040")); + + assertThat(bounds.boundingBox()).isEqualTo(box(1, 2, 1, 2)); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("invalidWkbCases") + public void testInvalidWkb(String description, String hexWkb, String expectedMessage) { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + bounds.add(decode("0101000000000000000000f03f0000000000000040")); + + assertThatThrownBy(() -> bounds.add(decode(hexWkb))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(expectedMessage); + assertThat(bounds.boundingBox()).isEqualTo(box(1, 2, 1, 2)); + } + + @Test + public void testDeeplyNestedWkbIsRejected() { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + + assertThatThrownBy(() -> bounds.add(nestedCollections(200))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("nesting too deep"); + } + + /** Returns WKB for a chain of geometry collections, each holding the next, around POINT(1 2). */ + private static ByteBuffer nestedCollections(int depth) { + ByteBuffer buffer = ByteBuffer.allocate(depth * 9 + 21).order(ByteOrder.LITTLE_ENDIAN); + for (int i = 0; i < depth; i += 1) { + buffer.put((byte) 1); + buffer.putInt(7); + buffer.putInt(1); + } + + buffer.put((byte) 1); + buffer.putInt(1); + buffer.putDouble(1.0); + buffer.putDouble(2.0); + buffer.flip(); + return buffer; + } + + private static Stream<Arguments> boundingBoxCases() { + return Stream.of( + Arguments.of("POINT EMPTY", "0101000000000000000000f87f000000000000f87f", null), + Arguments.of("POINT(1 2)", "0101000000000000000000f03f0000000000000040", box(1, 2, 1, 2)), + Arguments.of( + "POINT(1 2) big endian", "00000000013ff00000000000004000000000000000", box(1, 2, 1, 2)), + Arguments.of( + "LINESTRING(0 1,1 0,2 -1,-1 -2,0 1)", + "0102000000050000000000000000000000000000000000f03f000000000000f03f" + + "00000000000000000000000000000040000000000000f0bf000000000000f0bf" + + "00000000000000c00000000000000000000000000000f03f", + box(-1, -2, 2, 1)), + Arguments.of( + "POLYGON((0 0,10 0,0 10,0 0),(1 1,1 2,2 1,1 1))", + "010300000002000000040000000000000000000000000000000000000000000000" + + "000024400000000000000000000000000000000000000000000024400000000000" + + "000000000000000000000004000000000000000000f03f000000000000f03f0000" + + "00000000f03f00000000000000400000000000000040000000000000f03f000000" + + "000000f03f000000000000f03f", + box(0, 0, 10, 10)), + Arguments.of( + "MULTIPOINT((1 2),EMPTY,EMPTY,(3 4))", + "0104000000040000000101000000000000000000f03f000000000000004001010000" + + "00000000000000f87f000000000000f87f0101000000000000000000f87f00000000" + + "0000f87f000000000140080000000000004010000000000000", + box(1, 2, 3, 4)), + Arguments.of( + "MULTILINESTRING((1 2,3 4),(5 6,7 8))", + "010500000002000000010200000002000000000000000000f03f0000000000000040" + + "0000000000000840000000000000104001020000000200000000000000000014400000" + + "0000000018400000000000001c400000000000002040", + box(1, 2, 7, 8)), + Arguments.of( + "MULTIPOLYGON(EMPTY,((0 0,10 0,0 10,0 0),(1 1,1 2,2 1,1 1)))", + "01060000000200000001030000000000000001030000000200000004000000000000000000000000000" + + "00000000000000000000000244000000000000000000000000000000000000000000000244000000000" + + "00000000000000000000000004000000000000000000f03f000000000000f03f000000000000f03f000" + + "00000000000400000000000000040000000000000f03f000000000000f03f000000000000f03f", + box(0, 0, 10, 10)), + Arguments.of( + "GEOMETRYCOLLECTION(POINT(1 2),LINESTRING EMPTY,POLYGON EMPTY," + + "MULTIPOINT EMPTY,MULTILINESTRING EMPTY,MULTIPOLYGON EMPTY," + + "GEOMETRYCOLLECTION(POINT EMPTY,LINESTRING EMPTY,POLYGON EMPTY," + + "MULTIPOINT EMPTY,MULTILINESTRING EMPTY,MULTIPOLYGON EMPTY))", + "0107000000070000000101000000000000000000f03f000000000000004001020000" + + "00000000000103000000000000000104000000000000000105000000000000000106" + + "000000000000000107000000060000000101000000000000000000f87f0000000000" + + "00f87f01020000000000000001030000000000000001040000000000000001050000" + + "0000000000010600000000000000", + box(1, 2, 1, 2))); + } + + private static Stream<Arguments> invalidWkbCases() { + return Stream.of( + Arguments.of( + "trailing data", "01010000000000000000000840000000000000104000", "trailing data"), + Arguments.of( + "invalid multi-point child", + "010400000001000000010200000000000000", + "expected geometry type"), + Arguments.of( + "unsupported Z geometry", + "01e9030000000000000000f03f00000000000000400000000000000840", + "only 2D geometries are supported"), + Arguments.of( + "unsupported M geometry", + "01d1070000000000000000f03f00000000000000400000000000000840", + "only 2D geometries are supported"), + Arguments.of( + "unsupported ZM geometry", + "01b90b0000000000000000f03f000000000000004000000000000008400000000000001040", + "only 2D geometries are supported"), Review Comment: Updated: `extraDimensionsAreIgnored` asserts the XY box for POINT Z, POINT M, POINT ZM and LINESTRING Z, plus `extraDimensionsNestedInCollectionAreIgnored` for a 3D child inside a collection and `boundsAcrossValuesWithDifferentDimensions` across values of differing dimensionality. ########## core/src/test/java/org/apache/iceberg/TestGeometryBoundsCollector.java: ########## @@ -0,0 +1,196 @@ +/* + * 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.geospatial; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +public class TestGeometryBoundsCollector { + + @ParameterizedTest(name = "{0}") + @MethodSource("boundingBoxCases") + public void testBoundingBox(String wkt, String hexWkb, BoundingBox expected) { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + ByteBuffer wkb = decode(hexWkb); + int position = wkb.position(); + int limit = wkb.limit(); + + bounds.add(wkb); + + assertThat(wkb.position()).as(wkt).isEqualTo(position); + assertThat(wkb.limit()).as(wkt).isEqualTo(limit); + assertThat(bounds.boundingBox()).as(wkt).isEqualTo(expected); + } + + @Test + public void testNoBoundsWhenOneDimensionIsMissing() { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + bounds.add(decode("0101000000000000000000f03f000000000000f87f")); + + assertThat(bounds.boundingBox()).as("POINT(1 NaN)").isNull(); + } + + @Test + public void testBoundsAcrossValuesWithMissingCoordinates() { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + bounds.add(decode("0101000000000000000000f03f000000000000f87f")); + bounds.add(decode("0101000000000000000000f87f0000000000000040")); + + assertThat(bounds.boundingBox()).isEqualTo(box(1, 2, 1, 2)); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("invalidWkbCases") + public void testInvalidWkb(String description, String hexWkb, String expectedMessage) { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + bounds.add(decode("0101000000000000000000f03f0000000000000040")); + + assertThatThrownBy(() -> bounds.add(decode(hexWkb))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(expectedMessage); + assertThat(bounds.boundingBox()).isEqualTo(box(1, 2, 1, 2)); + } + + @Test + public void testDeeplyNestedWkbIsRejected() { + GeometryBoundsCollector bounds = new GeometryBoundsCollector(); + + assertThatThrownBy(() -> bounds.add(nestedCollections(200))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("nesting too deep"); + } + + /** Returns WKB for a chain of geometry collections, each holding the next, around POINT(1 2). */ + private static ByteBuffer nestedCollections(int depth) { + ByteBuffer buffer = ByteBuffer.allocate(depth * 9 + 21).order(ByteOrder.LITTLE_ENDIAN); + for (int i = 0; i < depth; i += 1) { + buffer.put((byte) 1); + buffer.putInt(7); + buffer.putInt(1); + } + + buffer.put((byte) 1); + buffer.putInt(1); + buffer.putDouble(1.0); + buffer.putDouble(2.0); + buffer.flip(); + return buffer; + } + + private static Stream<Arguments> boundingBoxCases() { + return Stream.of( + Arguments.of("POINT EMPTY", "0101000000000000000000f87f000000000000f87f", null), + Arguments.of("POINT(1 2)", "0101000000000000000000f03f0000000000000040", box(1, 2, 1, 2)), + Arguments.of( + "POINT(1 2) big endian", "00000000013ff00000000000004000000000000000", box(1, 2, 1, 2)), + Arguments.of( + "LINESTRING(0 1,1 0,2 -1,-1 -2,0 1)", + "0102000000050000000000000000000000000000000000f03f000000000000f03f" + + "00000000000000000000000000000040000000000000f0bf000000000000f0bf" + + "00000000000000c00000000000000000000000000000f03f", + box(-1, -2, 2, 1)), + Arguments.of( + "POLYGON((0 0,10 0,0 10,0 0),(1 1,1 2,2 1,1 1))", + "010300000002000000040000000000000000000000000000000000000000000000" + + "000024400000000000000000000000000000000000000000000024400000000000" + + "000000000000000000000004000000000000000000f03f000000000000f03f0000" + + "00000000f03f00000000000000400000000000000040000000000000f03f000000" + + "000000f03f000000000000f03f", + box(0, 0, 10, 10)), + Arguments.of( + "MULTIPOINT((1 2),EMPTY,EMPTY,(3 4))", + "0104000000040000000101000000000000000000f03f000000000000004001010000" + + "00000000000000f87f000000000000f87f0101000000000000000000f87f00000000" + + "0000f87f000000000140080000000000004010000000000000", + box(1, 2, 3, 4)), + Arguments.of( + "MULTILINESTRING((1 2,3 4),(5 6,7 8))", + "010500000002000000010200000002000000000000000000f03f0000000000000040" + + "0000000000000840000000000000104001020000000200000000000000000014400000" + + "0000000018400000000000001c400000000000002040", + box(1, 2, 7, 8)), + Arguments.of( + "MULTIPOLYGON(EMPTY,((0 0,10 0,0 10,0 0),(1 1,1 2,2 1,1 1)))", + "01060000000200000001030000000000000001030000000200000004000000000000000000000000000" + + "00000000000000000000000244000000000000000000000000000000000000000000000244000000000" + + "00000000000000000000000004000000000000000000f03f000000000000f03f000000000000f03f000" + + "00000000000400000000000000040000000000000f03f000000000000f03f000000000000f03f", + box(0, 0, 10, 10)), + Arguments.of( + "GEOMETRYCOLLECTION(POINT(1 2),LINESTRING EMPTY,POLYGON EMPTY," + + "MULTIPOINT EMPTY,MULTILINESTRING EMPTY,MULTIPOLYGON EMPTY," + + "GEOMETRYCOLLECTION(POINT EMPTY,LINESTRING EMPTY,POLYGON EMPTY," + + "MULTIPOINT EMPTY,MULTILINESTRING EMPTY,MULTIPOLYGON EMPTY))", + "0107000000070000000101000000000000000000f03f000000000000004001020000" + + "00000000000103000000000000000104000000000000000105000000000000000106" + + "000000000000000107000000060000000101000000000000000000f87f0000000000" + + "00f87f01020000000000000001030000000000000001040000000000000001050000" + + "0000000000010600000000000000", + box(1, 2, 1, 2))); + } + + private static Stream<Arguments> invalidWkbCases() { + return Stream.of( + Arguments.of( + "trailing data", "01010000000000000000000840000000000000104000", "trailing data"), + Arguments.of( + "invalid multi-point child", + "010400000001000000010200000000000000", + "expected geometry type"), + Arguments.of( + "unsupported Z geometry", + "01e9030000000000000000f03f00000000000000400000000000000840", + "only 2D geometries are supported"), + Arguments.of( + "unsupported M geometry", + "01d1070000000000000000f03f00000000000000400000000000000840", + "only 2D geometries are supported"), + Arguments.of( + "unsupported ZM geometry", + "01b90b0000000000000000f03f000000000000004000000000000008400000000000001040", + "only 2D geometries are supported"), + Arguments.of("truncated point", "0101000000", "unexpected end of buffer")); + } + + private static GeospatialBound xy(double xCoord, double yCoord) { + return GeospatialBound.createXY(xCoord, yCoord); + } + + private static BoundingBox box(double minX, double minY, double maxX, double maxY) { + return new BoundingBox(xy(minX, minY), xy(maxX, maxY)); + } + + private static ByteBuffer decode(String hex) { + byte[] bytes = new byte[hex.length() / 2]; + for (int i = 0; i < bytes.length; i += 1) { + int offset = i * 2; + bytes[i] = (byte) Integer.parseInt(hex.substring(offset, offset + 2), 16); + } + + return ByteBuffer.wrap(bytes); + } Review Comment: Added as `boundsFromBufferWithOffset`: a `slice()` at position 11 of a larger array, asserting both the box and that the caller's position is untouched. -- 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]
