Skip to content

fix(storage): preserve Parquet schema annotations - #20424

Open
youngsofun wants to merge 1 commit into
databendlabs:mainfrom
youngsofun:codex/fix-parquet-schema-roundtrip
Open

fix(storage): preserve Parquet schema annotations#20424
youngsofun wants to merge 1 commit into
databendlabs:mainfrom
youngsofun:codex/fix-parquet-schema-roundtrip

Conversation

@youngsofun

@youngsofun youngsofun commented Sep 1, 2026

Copy link
Copy Markdown
Member

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

Parquet table plans serialized their inferred schema by printing and reparsing the Parquet message type. That round-trip could normalize a legacy Decimal annotation from converted_type=DECIMAL, logical_type=None into an explicit Decimal logical type. Row-group readers then rejected the original file because its schema no longer exactly matched the plan.

This PR:

  • serializes the Parquet schema through its binary metadata representation so legacy annotations are preserved exactly
  • reports the first concrete Parquet schema difference, including the actual file and schema-inference source paths, while treating producer-specific root message names and field IDs as compatible for stage reads
  • reports field-count mismatches with both file-only and inferred-only field names; each list shows up to five names and its remaining count instead of dumping the full schema
  • adds direct schema-bytes and full plan serde round-trip tests that verify a legacy Decimal annotation remains unchanged
  • adds logic and unit coverage for compatible field IDs, readable first-difference diagnostics, field-list direction, and truncation

Mixed-version query nodes retain the existing Arrow-derived fallback when they cannot decode the other plan format. That fallback is best-effort and may normalize legacy annotations; subsequent schema mismatches now identify this condition and suggest checking query-node versions. Rolling-upgrade correctness for this case is outside this PR's scope.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Validated with focused catalog serialization tests, Parquet storage schema-difference tests, clippy for the affected crates, formatting, shell syntax, and diff checks. The schema-difference logic test reuses existing Parquet fixtures to cover file-only/inferred-only direction, complete short lists, and truncated wide-schema lists.

This regression does not keep an integration fixture. A tiny single-file stage read does not guarantee that the scan plan is serialized to another query node, so it can pass with the old implementation. Using several tiny files would still depend on scheduler placement rather than deterministically exercising the faulty round-trip. The direct schema-bytes round-trip and full ParquetTableInfo serde tests instead reproduce the normalization at its source and assert that logical_type=None is preserved.

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

AI assistance

  • AI usage: An AI coding agent investigated the Parquet schema round-trip, drafted the storage fix and regression tests, and assisted with validation and the PR summary
  • Responsible human: @youngsofun
  • The responsible human has read every line of this diff and can explain each change

This change is Reviewable

@github-actions github-actions Bot added the pr-bugfix this PR patches a bug in codebase label Sep 1, 2026
@youngsofun
youngsofun marked this pull request as draft September 2, 2026 01:41
@youngsofun
youngsofun force-pushed the codex/fix-parquet-schema-roundtrip branch 12 times, most recently from 4f3b33f to 818dce2 Compare September 7, 2026 08:38
@youngsofun
youngsofun force-pushed the codex/fix-parquet-schema-roundtrip branch from 818dce2 to 9d2193b Compare September 7, 2026 08:46
@youngsofun
youngsofun marked this pull request as ready for review September 7, 2026 10:17
@youngsofun
youngsofun requested review from KKould and b41sh September 8, 2026 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix this PR patches a bug in codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant