Skip to content

Core: Read geometry and geography bounds as GeospatialBound #18394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stevenzwu wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Core: Read geometry and geography bounds as GeospatialBound #18394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3187a8
Core: Read geometry and geography bounds as GeospatialBound
stevenzwu Oct 6, 2026
3de75a4
Core: Drop the generic StructLike bound copy for geospatial stats
stevenzwu Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 52 additions & 5 deletions api/src/main/java/org/apache/iceberg/geospatial/GeospatialBound.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
*/
package org.apache.iceberg.geospatial;

import java.io.Serializable;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.util.Objects;
import org.apache.iceberg.StructLike;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;

/**
Expand All @@ -44,7 +46,7 @@
* <p>This class represents a lower or upper geospatial bound and handles serialization and
* deserialization of these bounds to/from byte arrays, conforming to the Iceberg specification.
*/
public class GeospatialBound {
public class GeospatialBound implements StructLike, Serializable {
/**
* Parses a geospatial bound from a byte buffer according to Iceberg spec.
*
Expand Down Expand Up @@ -204,16 +206,20 @@ public static GeospatialBound createXYM(double x, double y, double m) {
}

@SuppressWarnings("MemberName")
private final double x;
private double x;

@SuppressWarnings("MemberName")
private final double y;
private double y;

@SuppressWarnings("MemberName")
private final double z;
private double z;

@SuppressWarnings("MemberName")
private final double m;
private double m;

private GeospatialBound() {
this(Double.NaN, Double.NaN, Double.NaN, Double.NaN);
}

/** Private constructor - use factory methods instead. */
@SuppressWarnings("ParameterName")
Expand Down Expand Up @@ -282,6 +288,47 @@ public boolean hasM() {
return !Double.isNaN(m);
}

/** Returns a new bound with the same coordinates. */
public GeospatialBound copy() {
return new GeospatialBound(x, y, z, m);
}

@Override
public int size() {
return 4;
}

@Override
public <T> T get(int pos, Class<T> javaClass) {
return switch (pos) {
case 0 -> javaClass.cast(x);
case 1 -> javaClass.cast(y);
case 2 -> hasZ() ? javaClass.cast(z) : null;
case 3 -> hasM() ? javaClass.cast(m) : null;
default -> throw new IllegalArgumentException("Invalid position: " + pos);
};
}

@Override
public <T> void set(int pos, T value) {
switch (pos) {
case 0 -> this.x = requiredCoordinate("x", value);
case 1 -> this.y = requiredCoordinate("y", value);
case 2 -> this.z = optionalCoordinate(value);
case 3 -> this.m = optionalCoordinate(value);
default -> throw new IllegalArgumentException("Invalid position: " + pos);
}
}

private static double requiredCoordinate(String coordinate, Object value) {
Preconditions.checkArgument(value != null, "Invalid %s coordinate: null", coordinate);
return (Double) value;
}

private static double optionalCoordinate(Object value) {
return value == null ? Double.NaN : (Double) value;
}

@Override
public String toString() {
return "GeospatialBound(" + simpleString() + ")";
Expand Down
71 changes: 71 additions & 0 deletions api/src/test/java/org/apache/iceberg/geospatial/TestGeospatialBound.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
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 org.apache.iceberg.StructLike;
import org.apache.iceberg.util.ByteBuffers;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -71,6 +73,75 @@ public void testCreateXYZM() {
assertThat(bound.hasM()).isTrue();
}

@Test
public void testStructLikeXY() {
StructLike bound = GeospatialBound.createXY(1.0, 2.0);
assertThat(bound.size()).isEqualTo(4);
assertThat(bound.get(0, Double.class)).isEqualTo(1.0);
assertThat(bound.get(1, Double.class)).isEqualTo(2.0);
assertThat(bound.get(2, Double.class)).isNull();
assertThat(bound.get(3, Double.class)).isNull();
}

@Test
public void testStructLikeXYZM() {

@nastra nastra Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe also test XYM and XYZ as the other tests do

StructLike bound = GeospatialBound.createXYZM(3.0, 4.0, 5.0, 6.0);
assertThat(bound.size()).isEqualTo(4);
assertThat(bound.get(0, Double.class)).isEqualTo(3.0);
assertThat(bound.get(1, Double.class)).isEqualTo(4.0);
assertThat(bound.get(2, Double.class)).isEqualTo(5.0);
assertThat(bound.get(3, Double.class)).isEqualTo(6.0);
}

@Test
public void testSet() {
GeospatialBound bound = GeospatialBound.createXY(1.0, 2.0);
bound.set(0, 9.0);
bound.set(1, 8.0);
bound.set(2, 7.0);
bound.set(3, 6.0);
assertThat(bound.get(0, Double.class)).isEqualTo(9.0);
assertThat(bound.get(1, Double.class)).isEqualTo(8.0);
assertThat(bound.get(2, Double.class)).isEqualTo(7.0);
assertThat(bound.get(3, Double.class)).isEqualTo(6.0);
assertThat(bound.x()).isEqualTo(9.0);
assertThat(bound.y()).isEqualTo(8.0);
assertThat(bound.z()).isEqualTo(7.0);
assertThat(bound.m()).isEqualTo(6.0);

bound.set(2, null);
bound.set(3, null);
assertThat(bound.get(2, Double.class)).isNull();
assertThat(bound.get(3, Double.class)).isNull();
assertThat(bound.hasZ()).isFalse();
assertThat(bound.hasM()).isFalse();
}

@Test
public void testSetRejectsNullRequiredCoordinate() {
GeospatialBound bound = GeospatialBound.createXY(1.0, 2.0);
assertThatThrownBy(() -> bound.set(0, null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Invalid x coordinate: null");
assertThatThrownBy(() -> bound.set(1, null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Invalid y coordinate: null");
}

@Test
public void testCopyXY() {
GeospatialBound bound = GeospatialBound.createXY(1.0, 2.0);
GeospatialBound copy = bound.copy();
assertThat(copy).isEqualTo(bound).isNotSameAs(bound);
}

@Test
public void testCopyXYZM() {
GeospatialBound bound = GeospatialBound.createXYZM(1.0, 2.0, 3.0, 4.0);
GeospatialBound copy = bound.copy();
assertThat(copy).isEqualTo(bound).isNotSameAs(bound);
}

@Test
public void testEqualsAndHashCode() {
GeospatialBound xy1 = GeospatialBound.createXY(1.0, 2.0);
Expand Down
7 changes: 4 additions & 3 deletions core/src/main/java/org/apache/iceberg/FieldStatsStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.List;
import org.apache.iceberg.geospatial.GeospatialBound;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.types.Type;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.ByteBuffers;
import org.apache.iceberg.util.StructLikeUtil;

class FieldStatsStruct<T> implements FieldStats<T>, StructLike, Serializable {
private final Types.StructType struct;
Expand Down Expand Up @@ -231,8 +231,9 @@ private static int[] posToOffset(Types.StructType struct) {
private static Object copyBound(Object bound) {
if (bound instanceof byte[] bytes) {
return copyOf(bytes);
} else if (bound instanceof StructLike struct) {

@stevenzwu stevenzwu Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the usage PartitionData and updated serialization test in TestFieldStatsStruct so that we can remove this copy branch.

return StructLikeUtil.copy(struct);
} else if (bound instanceof GeospatialBound geo) {
// readers reuse one bound instance
return geo.copy();
}

return bound;
Expand Down
31 changes: 31 additions & 0 deletions core/src/main/java/org/apache/iceberg/V4ManifestReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.iceberg.expressions.Expressions;
import org.apache.iceberg.expressions.InclusiveStatsEvaluator;
import org.apache.iceberg.expressions.Projections;
import org.apache.iceberg.geospatial.GeospatialBound;
import org.apache.iceberg.io.CloseableGroup;
import org.apache.iceberg.io.CloseableIterable;
import org.apache.iceberg.io.CloseableIterator;
Expand All @@ -40,6 +41,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
import org.apache.iceberg.relocated.com.google.common.collect.Sets;
import org.apache.iceberg.types.RestoreColumns;
import org.apache.iceberg.types.Type;
import org.apache.iceberg.types.TypeUtil;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.ArrayUtil;
Expand Down Expand Up @@ -69,6 +71,7 @@ static Builder uncommitted(
private final ManifestFile manifest;
private final ManifestBitmap dv;
private final FileIO io;
private final Schema tableSchema;
private final Schema readSchema;
private final String tableLocation;
private final InclusiveStatsEvaluator statsFilter;
Expand All @@ -83,6 +86,7 @@ static Builder uncommitted(
private V4ManifestReader(
ManifestFile manifest,
FileIO io,
Schema tableSchema,
Schema readSchema,
String tableLocation,
InclusiveStatsEvaluator statsFilter,
Expand All @@ -94,6 +98,7 @@ private V4ManifestReader(
this.manifest = manifest;
this.dv = manifest.manifestDeletionVector();
this.io = io;
this.tableSchema = tableSchema;
this.readSchema = readSchema;
this.tableLocation = tableLocation;
this.statsFilter = statsFilter;
Expand Down Expand Up @@ -240,6 +245,7 @@ private CloseableIterable<TrackedFile> open() {
readBuilder.setCustomType(TrackedFile.CONTENT_STATS_ID, ContentStatsStruct.class);
for (Types.NestedField fieldStats : statsField.type().asStructType().fields()) {
readBuilder.setCustomType(fieldStats.fieldId(), FieldStatsStruct.class);
registerGeospatialFieldStatsBounds(readBuilder, fieldStats);
}
}

Expand All @@ -248,6 +254,30 @@ private CloseableIterable<TrackedFile> open() {
return reader;
}

private void registerGeospatialFieldStatsBounds(
InternalData.ReadBuilder readBuilder, Types.NestedField fieldStats) {
Types.NestedField tableField = tableSchema.findField(StatsUtil.toFieldId(fieldStats.fieldId()));

@nastra nastra Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need the table schema for that. The stats schema should already have that information.
Essentially only GEO lower/upper fields are structs, so I think we could fetch that info from the Stats schema rather than having to pass table schema

if (tableField == null) {
return;
}

Type.TypeID typeId = tableField.type().typeId();
if (typeId != Type.TypeID.GEOMETRY && typeId != Type.TypeID.GEOGRAPHY) {
return;
}

Types.StructType struct = fieldStats.type().asStructType();
setGeospatialBoundCustomType(readBuilder, struct.field(StatsUtil.LOWER_BOUND_NAME));
setGeospatialBoundCustomType(readBuilder, struct.field(StatsUtil.UPPER_BOUND_NAME));
}

private static void setGeospatialBoundCustomType(
InternalData.ReadBuilder readBuilder, Types.NestedField boundField) {
if (boundField != null) {
readBuilder.setCustomType(boundField.fieldId(), GeospatialBound.class);
}
}

// resolves stored locations against the table location
private TrackedFile copyResolved(TrackedFile trackedFile) {
TrackedFileStruct copy =
Expand Down Expand Up @@ -427,6 +457,7 @@ V4ManifestReader build() {
return new V4ManifestReader(
manifest,
io,
tableSchema,
readSchema(!partitionFilters.isEmpty()),
tableLocation,
statsFilter(),
Expand Down
54 changes: 38 additions & 16 deletions core/src/test/java/org/apache/iceberg/TestFieldStatsStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.UUID;
import java.util.stream.Stream;
import org.apache.iceberg.TestHelpers.RoundTripSerializer;
import org.apache.iceberg.geospatial.GeospatialBound;
import org.apache.iceberg.inmemory.InMemoryOutputFile;
import org.apache.iceberg.io.CloseableIterable;
import org.apache.iceberg.io.FileAppender;
Expand Down Expand Up @@ -289,7 +290,19 @@ public void serializationAndCopy(

private static Stream<Arguments> geoCases() {
return ImmutableList.of(Types.GeometryType.crs84(), Types.GeographyType.crs84()).stream()
.flatMap(type -> SERIALIZERS.stream().map(serializer -> Arguments.of(type, serializer)));
.flatMap(
type ->
SERIALIZERS.stream()
.map(serializer -> Arguments.of(type, geoSerializer(serializer, type))));
}

private static Named<RoundTripSerializer<FieldStatsStruct<?>>> geoSerializer(
Named<RoundTripSerializer<FieldStatsStruct<?>>> serializer, Type type) {
if ("InternalData".equals(serializer.getName())) {
return Named.of("InternalData", stats -> roundTripInternalData(stats, type));
}

return serializer;
}

@ParameterizedTest
Expand All @@ -300,18 +313,8 @@ public void geoSerialization(Type geoType, RoundTripSerializer<FieldStatsStruct<
StatsUtil.fieldStatsStruct(geoType, BASE_ID, MetricsModes.Full.get());

// geometry and geography use bounding-box structs (x, y, z, m) for their bounds
PartitionData lowerBound =
new PartitionData(statsStruct.field("lower_bound").type().asStructType());
lowerBound.set(0, 1.0d);
lowerBound.set(1, 2.0d);
lowerBound.set(2, 3.0d);
lowerBound.set(3, 4.0d);
PartitionData upperBound =
new PartitionData(statsStruct.field("upper_bound").type().asStructType());
upperBound.set(0, 5.0d);
upperBound.set(1, 6.0d);
upperBound.set(2, 7.0d);
upperBound.set(3, 8.0d);
GeospatialBound lowerBound = GeospatialBound.createXYZM(1.0d, 2.0d, 3.0d, 4.0d);
GeospatialBound upperBound = GeospatialBound.createXYZM(5.0d, 6.0d, 7.0d, 8.0d);

FieldStatsStruct<Object> stats =
new FieldStatsStruct<>(statsStruct, lowerBound, upperBound, false, 28L, 2L, 0L, null);
Expand Down Expand Up @@ -376,6 +379,11 @@ public void variantSerialization(RoundTripSerializer<FieldStatsStruct<?>> serial

private static <T> FieldStatsStruct<T> roundTripInternalData(FieldStats<T> stats)
throws IOException {
return roundTripInternalData(stats, null);
}

private static <T> FieldStatsStruct<T> roundTripInternalData(FieldStats<T> stats, Type columnType)
throws IOException {
Schema schema = stats.type().asSchema();
InMemoryOutputFile file = new InMemoryOutputFile("internal.avro");

Expand All @@ -384,15 +392,29 @@ private static <T> FieldStatsStruct<T> roundTripInternalData(FieldStats<T> stats
writer.add(stats);
}

try (CloseableIterable<FieldStatsStruct<T>> reader =
InternalData.ReadBuilder readBuilder =
InternalData.read(FileFormat.AVRO, file.toInputFile())
.project(schema)
.setRootType(FieldStatsStruct.class)
.build()) {
.setRootType(FieldStatsStruct.class);
if (columnType != null
&& (columnType.typeId() == Type.TypeID.GEOMETRY
|| columnType.typeId() == Type.TypeID.GEOGRAPHY)) {
setGeospatialBoundCustomType(readBuilder, stats.type().field(StatsUtil.LOWER_BOUND_NAME));
setGeospatialBoundCustomType(readBuilder, stats.type().field(StatsUtil.UPPER_BOUND_NAME));
}

try (CloseableIterable<FieldStatsStruct<T>> reader = readBuilder.build()) {
return Iterables.getOnlyElement(reader);
}
}

private static void setGeospatialBoundCustomType(
InternalData.ReadBuilder readBuilder, Types.NestedField boundField) {
if (boundField != null) {
readBuilder.setCustomType(boundField.fieldId(), GeospatialBound.class);
}
}

private static int pos(Types.StructType type, String fieldName) {
List<Types.NestedField> fields = type.fields();
for (int pos = 0; pos < fields.size(); pos += 1) {
Expand Down
Loading
Loading