szehon-ho commented on code in PR #17509: URL: https://github.com/apache/iceberg/pull/17509#discussion_r3832516140
########## core/src/main/java/org/apache/iceberg/GeometryBoundsBuilder.java: ########## @@ -0,0 +1,407 @@ +/* + * 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>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 carried bytes beyond its declared geometry, meaning it was not fully parsed + // and 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>If the value carries bytes beyond the single geometry it declares, it was not fully parsed + * (for example a multi geometry whose declared element count is short), so an object never + * reached the bounds and the box can no longer cover the whole value. {@link #build()} then + * returns no bounds, since suppressing optional metrics is safe where under-covering a file is + * not. + * + * <p>If this throws, the builder's state is undefined: coordinates parsed before the failure may + * already be folded in, so a caller that continues after a rejected value must discard this + * builder. + * + * @param wkb a buffer containing one WKB geometry; trailing bytes suppress the bounds + * @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, ANY_GEOMETRY, ANY_DIMENSION); + 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 carried trailing bytes (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 IllegalArgumentException("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); + Preconditions.checkArgument( Review Comment: These three are valid input rather than corruption โ OGC 06-103r4 ยง8.2.8 assigns PolyhedralSurface=15, TIN=16 and Triangle=17 plus their 1015/2015/3015 variants, and Appendix G defines Iceberg geometry by reference to that document. Not supporting them is reasonable, but the rejection should land in the try/catch above and leave the file without bounds instead of failing the write. PolyhedralSurface and TIN are what 3D city and terrain models use, so a table fed by one of those pipelines wouldn't hit this occasionally โ every insert would fail. A WARN naming the type code when bounds are suppressed would help here, since it separates "valid but unsupported" from "corrupt" โ through the catch both just look like missing metrics. -- 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]
