Skip to content

Parquet: Validate geospatial projection parameters - #17578

Open
manuzhang wants to merge 3 commits into
apache:mainfrom
manuzhang:codex/validate-parquet-geospatial-types
Open

Parquet: Validate geospatial projection parameters#17578
manuzhang wants to merge 3 commits into
apache:mainfrom
manuzhang:codex/validate-parquet-geospatial-types

Conversation

@manuzhang

Copy link
Copy Markdown
Member

Summary

  • validate Parquet geometry and geography logical-type parameters against the projected Iceberg type
  • reuse the existing Parquet-to-Iceberg primitive conversion so omitted defaults and case-insensitive CRS matching remain consistent
  • add focused coverage for matching parameters, geometry and geography CRS mismatches, and geography edge-algorithm mismatches

Context

Parquet geospatial values carry their CRS and edge interpolation algorithm in the logical-type annotation rather than in the WKB payload. PruneColumns previously ignored these parameters, so a file could be interpreted using incompatible parameters from the projected Iceberg schema.

This is the Java-side counterpart discovered while reviewing apache/iceberg-cpp#880.

Testing

  • ./gradlew :iceberg-parquet:test --tests org.apache.iceberg.parquet.TestPruneColumns
  • ./gradlew :iceberg-parquet:spotlessJavaCheck
  • git diff --check

AI Disclosure

  • Model: GPT-5
  • Platform/Tool: Codex
  • Human Oversight: User requested the fix; this draft is awaiting human review.
  • Prompt Summary: Compare the Iceberg C++ geospatial reader behavior with Java, confirm whether Java has the same issue, and create a Java-side fix PR.

Copilot AI left a comment

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.

Pull request overview

Validates Parquet geospatial annotations against projected Iceberg types.

Changes:

  • Reuses primitive-type conversion for validation.
  • Rejects CRS and edge-algorithm mismatches.
  • Adds focused geospatial projection tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
PruneColumns.java Validates projected geospatial parameters.
MessageTypeToType.java Extracts reusable primitive conversion.
TestPruneColumns.java Tests matching and mismatched parameters.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread parquet/src/main/java/org/apache/iceberg/parquet/PruneColumns.java Outdated
@manuzhang
manuzhang requested a balanced review from Copilot August 9, 2026 15:31
@manuzhang
manuzhang marked this pull request as ready for review August 9, 2026 15:33

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

parquet/src/main/java/org/apache/iceberg/parquet/ParquetSchemaUtil.java:165

  • This validates only top-level primitives in the ID-less fallback path. When a selected top-level field is a struct, list, or map, the entire Parquet group is retained unchanged, so any projected nested geometry/geography still bypasses the new CRS/edge-algorithm check and can be read with incompatible parameters. Please traverse each matched group positionally against the projected type and validate its nested primitives as well; add a nested ID-less mismatch test to cover this path.
        Types.NestedField expectedField = expectedSchema.findField(ordinal);
        if (type.isPrimitive() && expectedField.type().isPrimitiveType()) {
          PruneColumns.validatePrimitive(
              expectedField.type().asPrimitiveType(), type.asPrimitiveType());

@manuzhang

Copy link
Copy Markdown
Member Author

@huan233usc Please take a look, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants