GH-50910: [Parquet] Tolerate unrecognized logical/physical type combinations when reading by divjotarora · Pull Request #50909 · apache/arrow

divjotarora · 2026-08-18T20:34:26Z

Rationale for this change

See apache/parquet-format#607 for rationale.

What changes are included in this PR?

This PR gracefully handles unrecognized logical/physical type combinations by dropping the logical type during the read and dropping any associated statistics for the relevant columns. Note that unrecognized logical types are already handled gracefully and no changes were required.

Are these changes tested?

Yes, via unit tests and an e2e test that reads a real file that contains an invalid type combination.

Are there any user-facing changes?

No

GitHub Issue: [Format] Tolerate unrecognized logical/physical type combinations when reading #50910

github-actions · 2026-08-18T20:34:51Z

Thanks for opening a pull request!

This pull request has been automatically converted to a draft because its title doesn't match Arrow's required format.

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

After updating the title, you can mark the pull request as ready for review.

See also:

github-actions · 2026-08-18T20:37:22Z

⚠️ GitHub issue #50910 has been automatically assigned in GitHub to PR creator.

Reranko05

Could you also add Component of the PR in the title, it will be easier for maintainers to review it.
GH-<Issue Number>: [<Component>] <Title>

emkornfield

a couple of questions, but seems reasonable.

emkornfield · 2026-09-23T23:31:35Z

+    // type annotation.
+    if (logical_type &&
+        !logical_type->is_applicable(physical_type, element->type_length)) {
+      ARROW_LOG(WARNING) << "Dropping unsupported logical type "


I think this might be the first log statement we have in Parquet code. @wgtmac @pitrou any concerns with adding it, it seems useful?

I think this logging is worth adding if the behavior is not configurable.

Hmm, I don't think I like the idea of logging "problems" like this, especially if the logging cannot be suppressed.

@emkornfield @wgtmac Any thoughts on this?

Also cc @HuaHuaY

The main purpose of this PR is to allow reading Parquet files that use features not yet implemented in Parquet C++ (a new logical/physical type combination).

We don't emit warnings for other unrecognized features, so I think it's reasonable to be silent here as well.

(warnings could come across as noisy, especially if the user cannot do anything about them)

I have no strong opinion here, but isn't it better for a user to know that they're using a library that doesn't understand the full semantic meaning of the files they're reading? The results are still correct, but the library is not doing any skipping/pruning that it might otherwise do and realistically they should upgrade it (or fix their files in case it's the data that's wrong for some reason).

I have no strong opinion here, but isn't it better for a user to know that they're using a library that doesn't understand the full semantic meaning of the files they're reading?

Either they care about those semantics and they will notice anyway (because the contents won't be read as the expected Arrow datatype: for example a TIMESTAMP FLBA(12) column would be read as FixedSizeBinary rather Timestamp), or they don't care about those semantics and the warning message is just a distraction.

realistically they should upgrade it

Only if there is a newer version of the library that supports said combination, and only if they can reasonably upgrade (perhaps they are using some application that links Parquet C++, and the upgrade cycle depends on the application's release cycle).

Dropped the logging and all related changes (e.g. schema path propagation in FromParquet) in 40e8e79.

@pitrou @emkornfield @wgtmac PTAL and let me know what you prefer

I'm OK dropping it. But I think it could add value one additional way the log is useful is as telemetry for people running online services. Lets keep it removed for now.

@pitrou any further concerns with this removed and the public API change gone?

emkornfield · 2026-09-23T23:32:16Z

Triggered workflows generally one question on logging that I'd like other input on, otherwise seems reasonable to be as long as CI passes.

wgtmac · 2026-09-24T02:01:50Z

+    // type annotation.
+    if (logical_type &&
+        !logical_type->is_applicable(physical_type, element->type_length)) {
+      ARROW_LOG(WARNING) << "Dropping unsupported logical type "


I think this logging is worth adding if the behavior is not configurable.

Co-authored-by: Isaac <no-reply@databricks.com>

divjotarora · 2026-09-25T19:53:05Z

  size_t pos = 0;
  size_t num_reserved = 0;

-  std::function<std::unique_ptr<Node>(int depth)> NextNode = [&](int depth) {


The majority of the remove/add changes here are due to whitespace because of the addition of a const SchemaPath* parent_path parameter.

emkornfield

Generally, LGTM, unless there are other comments, I think once other comments are addressed we can merge this (will aim to do so Monday unless there is additional feedback).

emkornfield

it would be nice if we could test the nested case to ensure the new path naming works as intended.

Co-authored-by: Isaac <no-reply@databricks.com>

divjotarora · 2026-09-26T20:19:41Z

it would be nice if we could test the nested case to ensure the new path naming works as intended.

Added a test to schema_test.cc

emkornfield · 2026-09-28T20:06:20Z

CI failure looks unrelated. Going to merge now (actually might be a little bit, need to reset up the merge script on my box).

pitrou · 2026-09-29T07:08:03Z

 public:
  static std::unique_ptr<Node> FromParquet(const void* opaque_element);
+  static std::unique_ptr<Node> FromParquet(const void* opaque_element,
+                                           const SchemaPath* parent_path);


Hmm, why are we exposing a public API that the user cannot call (because it expects an incomplete type)?

I added this so Unflatten could call it. Maybe let's resolve the logging discussion first as this function is only needed to do the logging?

If we decide to keep the logging, we could make this new overload private but still accessible from Unflatten:

PARQUET_EXPORT friend std::unique_ptr<Node> Unflatten( std::span<const format::SchemaElement> elements, int max_depth);

Let me know if you'd prefer this approach.

Co-authored-by: Isaac <no-reply@databricks.com>

divjotarora requested review from pitrou and wgtmac as code owners August 18, 2026 20:34

github-actions Bot marked this pull request as draft August 18, 2026 20:34

github-actions Bot added Component: Parquet Component: C++ awaiting review Awaiting review labels Aug 18, 2026

divjotarora changed the title ~~Tolerate unrecognized logical/physical type combinations when reading~~ GH-50910: Tolerate unrecognized logical/physical type combinations when reading Aug 18, 2026

divjotarora force-pushed the log-phys-type-combo branch from 4dc0ee3 to 6017b0e Compare August 18, 2026 21:03

divjotarora commented Aug 18, 2026

View reviewed changes

Comment thread cpp/submodules/parquet-testing

divjotarora force-pushed the log-phys-type-combo branch from 6017b0e to 7062580 Compare August 18, 2026 23:58

divjotarora marked this pull request as ready for review August 18, 2026 23:58

Reranko05 reviewed Aug 19, 2026

View reviewed changes

divjotarora changed the title ~~GH-50910: Tolerate unrecognized logical/physical type combinations when reading~~ GH-50910: [Core] Tolerate unrecognized logical/physical type combinations when reading Aug 19, 2026

divjotarora changed the title ~~GH-50910: [Core] Tolerate unrecognized logical/physical type combinations when reading~~ GH-50910: [Format] Tolerate unrecognized logical/physical type combinations when reading Aug 19, 2026

github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Aug 20, 2026

emkornfield changed the title ~~GH-50910: [Format] Tolerate unrecognized logical/physical type combinations when reading~~ GH-50910: [Parquet] Tolerate unrecognized logical/physical type combinations when reading Aug 20, 2026

emkornfield reviewed Aug 20, 2026

View reviewed changes

Comment thread cpp/src/parquet/schema.cc Outdated

emkornfield reviewed Aug 20, 2026

View reviewed changes

Comment thread cpp/src/parquet/schema.cc

emkornfield requested changes Aug 20, 2026

View reviewed changes

divjotarora requested a review from emkornfield August 20, 2026 16:24

gunnarmorling mentioned this pull request Sep 9, 2026

#1139 Tell a file that contradicts itself apart from a caller that asked for the wrong type hardwood-hq/hardwood#1140

Merged

emkornfield reviewed Sep 23, 2026

View reviewed changes

github-actions Bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Sep 23, 2026

wgtmac reviewed Sep 24, 2026

View reviewed changes

divjotarora requested a review from HuaHuaY as a code owner September 25, 2026 06:57

divjotarora requested a review from emkornfield September 25, 2026 07:10

emkornfield reviewed Sep 25, 2026

View reviewed changes

Comment thread cpp/src/parquet/schema.cc Outdated

emkornfield requested changes Sep 25, 2026

View reviewed changes

divjotarora and others added 5 commits September 25, 2026 19:24

Tolerate unrecognized logical/physical type combinations when reading

4050266

address comments

24e0931

Identify unsupported Parquet types by full column path

470a0d6

Co-authored-by: Isaac <no-reply@databricks.com>

Simplify access to the schema parsing overload

2780c4a

Co-authored-by: Isaac <no-reply@databricks.com>

Reuse schema traversal for diagnostic column paths

b136ffd

Co-authored-by: Isaac <no-reply@databricks.com>

divjotarora force-pushed the log-phys-type-combo branch from 3205b68 to b136ffd Compare September 25, 2026 19:28

Initialize schema traversal lambda directly

e907cda

Co-authored-by: Isaac <no-reply@databricks.com>

divjotarora requested a review from emkornfield September 25, 2026 19:51

divjotarora commented Sep 25, 2026

View reviewed changes

emkornfield reviewed Sep 25, 2026

View reviewed changes

github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Sep 25, 2026

Test nested incompatible logical type warning paths

6a4d0ad

Co-authored-by: Isaac <no-reply@databricks.com>

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Sep 26, 2026

divjotarora requested a review from emkornfield September 27, 2026 18:01

emkornfield approved these changes Sep 28, 2026

View reviewed changes

Comment thread cpp/src/parquet/schema_test.cc Outdated

github-actions Bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Sep 28, 2026

pitrou reviewed Sep 29, 2026

View reviewed changes

Remove incompatible logical type logging and path tracking

40e8e79

Co-authored-by: Isaac <no-reply@databricks.com>

divjotarora requested a review from pitrou September 30, 2026 08:06

github-actions Bot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels Sep 30, 2026

Conversation

divjotarora commented Aug 18, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

Uh oh!

Reranko05 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

emkornfield left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emkornfield commented Sep 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emkornfield left a comment

Choose a reason for hiding this comment

Uh oh!

emkornfield left a comment

Choose a reason for hiding this comment

Uh oh!

divjotarora commented Sep 26, 2026

Uh oh!

Uh oh!

emkornfield commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

divjotarora commented Aug 18, 2026 •

edited by github-actions Bot

Loading

emkornfield commented Sep 28, 2026 •

edited

Loading