Skip to content

Parquet: Validate geospatial projection parameters #17578

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
manuzhang wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Parquet: Validate geospatial projection parameters #17578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1360372
Parquet: Validate geospatial projection parameters
manuzhang Aug 9, 2026
a6bc8b5
Parquet: Validate fallback geospatial projections
manuzhang Aug 9, 2026
d1e612f
Parquet: Fix geospatial validation Checkstyle
manuzhang Aug 9, 2026
e86bf5a
Parquet: Assert mismatched geography algorithm
manuzhang Aug 10, 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
4 changes: 4 additions & 0 deletions parquet/src/main/java/org/apache/iceberg/parquet/MessageTypeToType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ public Type map(GroupType map, Type keyType, Type valueType) {

@Override
public Type primitive(PrimitiveType primitive) {
return convertPrimitive(primitive);
}

static Type convertPrimitive(PrimitiveType primitive) {
// first, use the logical type annotation, if present
LogicalTypeAnnotation logicalType = primitive.getLogicalTypeAnnotation();
if (logicalType != null) {
Expand Down
6 changes: 6 additions & 0 deletions parquet/src/main/java/org/apache/iceberg/parquet/ParquetSchemaUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ public static MessageType pruneColumnsFallback(MessageType fileSchema, Schema ex
int ordinal = 1;
for (Type type : fileSchema.getFields()) {
if (selectedIds.contains(ordinal)) {
Types.NestedField expectedField = expectedSchema.findField(ordinal);
if (type.isPrimitive() && expectedField.type().isPrimitiveType()) {
PruneColumns.validatePrimitive(
expectedField.type().asPrimitiveType(), type.asPrimitiveType());
}

builder.addField(type.withId(ordinal));
}
ordinal += 1;
Expand Down
15 changes: 15 additions & 0 deletions parquet/src/main/java/org/apache/iceberg/parquet/PruneColumns.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.apache.iceberg.relocated.com.google.common.base.Objects;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.types.Type.TypeID;
import org.apache.iceberg.types.TypeUtil;
import org.apache.iceberg.types.Types.ListType;
import org.apache.iceberg.types.Types.MapType;
import org.apache.iceberg.types.Types.NestedField;
Expand Down Expand Up @@ -162,9 +164,22 @@ public Type variant(
@Override
public Type primitive(
org.apache.iceberg.types.Type.PrimitiveType expected, PrimitiveType primitive) {
validatePrimitive(expected, primitive);
return null;
}

static void validatePrimitive(
org.apache.iceberg.types.Type.PrimitiveType expected, PrimitiveType primitive) {
if (expected != null
&& (expected.typeId() == TypeID.GEOMETRY || expected.typeId() == TypeID.GEOGRAPHY)) {
Preconditions.checkArgument(
TypeUtil.isPromotionAllowed(MessageTypeToType.convertPrimitive(primitive), expected),
"Cannot read Parquet type %s as Iceberg type %s",
primitive,
expected);
}
}

private Integer getId(Type type) {
return type.getId() == null ? null : type.getId().intValue();
}
Expand Down
110 changes: 110 additions & 0 deletions parquet/src/test/java/org/apache/iceberg/parquet/TestPruneColumns.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@
package org.apache.iceberg.parquet;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import org.apache.iceberg.Schema;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.types.EdgeAlgorithm;
import org.apache.iceberg.types.Types.DoubleType;
import org.apache.iceberg.types.Types.GeographyType;
import org.apache.iceberg.types.Types.GeometryType;
import org.apache.iceberg.types.Types.IntegerType;
import org.apache.iceberg.types.Types.ListType;
import org.apache.iceberg.types.Types.MapType;
Expand All @@ -31,6 +35,7 @@
import org.apache.iceberg.types.Types.StructType;
import org.apache.iceberg.types.Types.VariantType;
import org.apache.iceberg.variants.Variant;
import org.apache.parquet.column.schema.EdgeInterpolationAlgorithm;
import org.apache.parquet.schema.LogicalTypeAnnotation;
import org.apache.parquet.schema.MessageType;
import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName;
Expand Down Expand Up @@ -305,6 +310,111 @@ public void testVariant() {
assertThat(actual).as("Pruned schema should be matched").isEqualTo(expected);
}

@Test
public void acceptsMatchingGeospatialParameters() {
MessageType fileSchema =
Types.buildMessage()
.optional(PrimitiveTypeName.BINARY)
.as(LogicalTypeAnnotation.geometryType(null))
.id(1)
.named("geom_default")
.optional(PrimitiveTypeName.BINARY)
.as(LogicalTypeAnnotation.geometryType("EPSG:3857"))
.id(2)
.named("geom_projected")
.optional(PrimitiveTypeName.BINARY)
.as(LogicalTypeAnnotation.geographyType(null, null))
.id(3)
.named("geog_default")
.optional(PrimitiveTypeName.BINARY)
.as(
LogicalTypeAnnotation.geographyType(
"EPSG:4326", EdgeInterpolationAlgorithm.ANDOYER))
.id(4)
.named("geog_custom")
.named("table");

Schema projection =
new Schema(
NestedField.optional(1, "geom_default", GeometryType.crs84()),
NestedField.optional(2, "geom_projected", GeometryType.of("epsg:3857")),
NestedField.optional(3, "geog_default", GeographyType.crs84()),
NestedField.optional(
4, "geog_custom", GeographyType.of("epsg:4326", EdgeAlgorithm.ANDOYER)));

assertThat(ParquetSchemaUtil.pruneColumns(fileSchema, projection)).isEqualTo(fileSchema);
}

@Test
public void rejectsGeometryCrsMismatch() {
MessageType fileSchema =
Types.buildMessage()
.optional(PrimitiveTypeName.BINARY)
.as(LogicalTypeAnnotation.geometryType("EPSG:3857"))
.id(1)
.named("geom")
.named("table");
Schema projection = new Schema(NestedField.optional(1, "geom", GeometryType.crs84()));

assertThatThrownBy(() -> ParquetSchemaUtil.pruneColumns(fileSchema, projection))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Cannot read Parquet type")
.hasMessageContaining("geometry(OGC:CRS84)");
}

@Test
public void rejectsGeometryCrsMismatchWithoutIds() {
MessageType fileSchema =
Types.buildMessage()
.optional(PrimitiveTypeName.BINARY)
.as(LogicalTypeAnnotation.geometryType("EPSG:3857"))
.named("geom")
.named("table");
Schema projection = new Schema(NestedField.optional(1, "geom", GeometryType.crs84()));

assertThatThrownBy(() -> ParquetSchemaUtil.pruneColumnsFallback(fileSchema, projection))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Cannot read Parquet type")
.hasMessageContaining("geometry(OGC:CRS84)");
}

@Test
public void rejectsGeographyCrsMismatch() {
MessageType fileSchema =
Types.buildMessage()
.optional(PrimitiveTypeName.BINARY)
.as(
LogicalTypeAnnotation.geographyType(
"EPSG:4326", EdgeInterpolationAlgorithm.SPHERICAL))
.id(1)
.named("geog")
.named("table");
Schema projection = new Schema(NestedField.optional(1, "geog", GeographyType.crs84()));

assertThatThrownBy(() -> ParquetSchemaUtil.pruneColumns(fileSchema, projection))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Cannot read Parquet type")
.hasMessageContaining("geography(OGC:CRS84, spherical)");
}

@Test
public void rejectsGeographyAlgorithmMismatch() {
MessageType fileSchema =
Types.buildMessage()
.optional(PrimitiveTypeName.BINARY)
.as(LogicalTypeAnnotation.geographyType("OGC:CRS84", EdgeInterpolationAlgorithm.KARNEY))
.id(1)
.named("geog")
.named("table");
Schema projection = new Schema(NestedField.optional(1, "geog", GeographyType.crs84()));

assertThatThrownBy(() -> ParquetSchemaUtil.pruneColumns(fileSchema, projection))

@anoopj anoopj Aug 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From what I understood, the exception is thrown because the algorithm is different (ie EdgeInterPolationAlgorithm.KARNEY). So shouldn't we be looking for that in the error string?

.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Cannot read Parquet type")
.hasMessageContaining("KARNEY")
.hasMessageContaining("geography(OGC:CRS84, spherical)");
}

private static Type buildVariantType(int id, String name) {
return Types.buildGroup(Type.Repetition.OPTIONAL)
.as(LogicalTypeAnnotation.variantType(Variant.VARIANT_SPEC_VERSION))
Expand Down
Loading