Skip to content

GH-51303: [C++][Parquet] Update parquet.thrift to sync with 2.14.0 - #51304

Open
HuaHuaY wants to merge 1 commit into
apache:mainfrom
HuaHuaY:upgrade_parquet
Open

GH-51303: [C++][Parquet] Update parquet.thrift to sync with 2.14.0#51304
HuaHuaY wants to merge 1 commit into
apache:mainfrom
HuaHuaY:upgrade_parquet

Conversation

@HuaHuaY

@HuaHuaY HuaHuaY commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Apache Parquet has just released its format version 2.14.0, which supports ALP, FILE type and some other features. We need to update parquet.thrift and its generated source files to be in sync.

What changes are included in this PR?

  1. Update parquet.thrift and its generated source files.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

Copilot AI lite review requested due to automatic review settings September 11, 2026 11:18
@github-actions github-actions Bot added the awaiting review Awaiting review label Sep 11, 2026

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.

🟡 Changes recommended

ALP collides with the internal UNDEFINED encoding sentinel, preventing correct conversion and round-tripping.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Synchronizes Parquet’s C++ Thrift schema and generated bindings with format 2.14.0.

Changes:

  • Adds FILE logical type, ALP encoding, and INT96 timestamp ordering.
  • Regenerates C++ Thrift serialization and type-support sources.
File summaries
File Summary
cpp/src/parquet/parquet.thrift Defines Parquet 2.14.0 schema additions.
cpp/src/generated/parquet_types.tcc Adds generated serialization logic.
cpp/src/generated/parquet_types.h Adds generated declarations and enum values.
cpp/src/generated/parquet_types.cpp Adds generated implementations and enum mappings.
Review details
  • Files reviewed: 1/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

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

Copilot AI review requested due to automatic review settings September 11, 2026 11:29

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.

🟡 Changes recommended

EncodingToString lacks a mapping for Encoding::ALP.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 2/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread cpp/src/parquet/types.h Outdated
Copilot AI review requested due to automatic review settings September 11, 2026 11:35

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.

🔵 Needs a closer look

Add the missing Encoding::ALP string mapping and regression test.

Review details

Suppressed comments (1)

cpp/src/parquet/types.h:542

  • ALP is now a valid public encoding, but EncodingToString() still has no case Encoding::ALP (cpp/src/parquet/types.cc:250-272). As a result, printer output and diagnostics for ALP metadata report UNKNOWN instead of ALP; please add the mapping (and a regression test) with this enum update.
    ALP = 10,
  • Files reviewed: 2/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 11, 2026 11:42
@HuaHuaY

HuaHuaY commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

I reverted the changes to Encoding::type. Since I didn't modify functions like EncodingToString in types.cc, leaving things in a half-finished state felt odd.

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.

🟡 Changes recommended

The ALP enum-value conflict causes incorrect metadata conversion and requires updates to the mirrored enum, sentinel, string conversion, and tests.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 1/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

bit-packing; values that cannot be converted losslessly are stored as
exceptions. See Encodings.md for the detailed specification.
*/
ALP = 10;

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 think this is acceptable.

@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants