[VL] Fix Parquet metadata scans with same-named data columns by felipepessoto · Pull Request #13136 · apache/gluten

felipepessoto · 2026-09-25T09:11:19Z

What changes are proposed in this pull request?

Fix a valid metadata-only query failing because a user column happens to have the same name as a metadata field:

CREATE TABLE metadata_collision (file_name BIGINT) USING parquet;
INSERT INTO metadata_collision VALUES (1), (2);
SELECT DISTINCT _metadata.file_name FROM metadata_collision;

Expected: the names of the data files, not the stored values 1 and 2.
Before this fix: the native reader fails with:

Converted type BIGINT is not allowed for requested type VARCHAR for file column 'file_name'

Why: _metadata.file_name is a synthetic string supplied for each file. For scans without an explicit table schema, Gluten reused the complete scan-output schema as the physical reader schema. Parquet therefore tried to read the unrelated physical file_name BIGINT as VARCHAR, before the metadata constant could be supplied. The query contains no cast.

Fix: build the derived physical schema from regular column roles only. Metadata remains in the scan output but no longer requests a physical field; this query has an empty physical projection. The rule covers all six built-in file constants, not just file_name.

Explicit physical schemas and column ordering, filter binding, and the existing fallback when both same-named real and metadata fields are output remain unchanged. No cast or Scala/Substrait protocol changes. Direct timestamp-metadata filter pushdown is outside scope.

How was this patch tested?

Current head 27e9bf905: 82 CI checks passed, 2 skipped, with no failed or pending checks.
Delta 4.4.1 / Spark 4.1 passed in run 40763107194. All 12 removed generated-suite cases executed and passed in the JUnit reports; all eight shard gates and aggregation passed with zero regressions, stale entries, or now-passing baseline entries.
95/95 metadata tests passed locally for the latest test-only cleanup using source-matched native binaries from this PR. Coverage checks actual values and native scans across all six constants, aliases, empty files, filters, partitions, and positional mapping.
Earlier revisions also passed 111/111 metadata/row-index tests and demonstrated the same file_name reproduction failing with baseline libraries and passing with this fix. Those results predate the latest rebase.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: GitHub Copilot 1.0.87-0 (GPT-6 Astra)

felipepessoto · 2026-09-25T17:08:10Z

/delta-test

github-actions · 2026-09-25T17:08:18Z

🔄 Delta Spark UT started by @felipepessoto (~2.5 h). View run

felipepessoto · 2026-09-26T01:45:22Z

@philo-he @zhouyuan @zhztheplayer could you please review this PR? Thank you!

Copilot

Copilot review overview

🟢 Approval recommended

No unresolved approval-blocking issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

Fixes metadata-only Parquet scans when synthetic metadata names collide with physical columns.

Changes:

Separates logical metadata outputs from derived physical reader schemas.
Preserves explicit schemas, filters, and positional mapping.
Adds Spark/native regressions and removes 12 fixed Delta failures.

File	Reviewed change
`gluten-ut/spark41/src/test/scala/org/apache/spark/sql/execution/datasources/GlutenFileMetadataStructSuite.scala`	Adds Spark metadata collision and mapping regressions.
`cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc`	Adds native schema and execution tests.
`cpp/velox/substrait/SubstraitToVeloxPlan.cc`	Separates logical scan and physical reader schemas.
`.github/workflows/util/delta-spark-ut/known-failures.txt`	Removes verified fixed Delta cases.

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

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

felipepessoto · 2026-10-15T23:30:12Z

@zhztheplayer could you please review this PR? The rebase onto main is complete, and all 90 CI checks pass on de5a5cd. Thank you!

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

4 open issues

The Parquet files are written under tmpDir_->getPath() + name (see writeParquet), but… · New
The test relies on a strict < 1 MiB file size assertion to ensure `FILES_MAX_PARTITION_BYTES =… · New
This loop compares int to functions.size() (a size_t), which can trigger signed/unsigned… · New
scala.collection.JavaConverters._ is deprecated in Scala 2.13+ in favor of… · New

_{Review effort: Lite}

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

writeParquet() always prefixes tmpDir_->getPath() to name. Several tests pass values like… · New
The Parquet reader factory is global process state. Unregistering it after… · New
Setting length to uint64_t::max() can distort split sizing logic or any code that uses the… · New

4 resolved since last review

_{Review effort: Lite}

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

writeParquet()writes totmpDir_->getPath() + name, but makeParquetFiles()` is being given… · New
writeParquet()writes totmpDir_->getPath() + name, but makeParquetFiles()` is being given… · New
schemaWithMetadataCollisionsrelies onzipWithIndex ordering (index < 2`) to decide types. This… · New

3 resolved since last review

_{Review effort: Lite}

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

7 open issues

The PR description says the 12 removed entries use renamed Delta 4.4.1 suite identities, and that… · New
The comment on the explicit-schema branch says physical fields and their ordinals must be… · New
The two branches each build dataColumns with a separate loop and similar name and type collection… · New
The loop variable copies each element of the columnTypes vector. Iterate by const reference (`for… · New
Setting the file length to the maximum uint64_t value is an unexplained magic value. Add a… · New
The 1 MB limit is repeated as a literal in two forms (1024 * 1024 and \"1m\"). They must stay… · New
The index 6 is an unexplained magic number for the id column. It is repeated in several tests… · New

_{Review effort: Lite}

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

felipepessoto · 2026-10-28T23:18:47Z

@rui-mo, @zhouyuan, @zhztheplayer, could you please review this PR? It fixes Parquet metadata scans failing when metadata fields share names with physical columns. Thanks!

rui-mo

Thanks for the fix!

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Derive reader data columns from regular scan slots without changing logical output or filter binding. Preserve explicit physical schemas and existing collision fallback, with native and Spark metadata regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Disable native fixture writes and verify both empty files have readable Parquet footers, zero rows, and the conflicting INT64 field. Avoid the native writer's zero-byte output for empty input. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Serialize scan mapping modes through split metadata instead of using an ignored connector option. Assert mode forwarding and distinguish positional mapping from name mapping. Resolve the overloaded Parquet footer type lookup in the Spark empty-file test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Resolve Spark input-file URIs before checking empty Parquet footers. Exercise a nontrivial native residual timestamp predicate while retaining string/numeric pushdown coverage; direct synthesized timestamp equality is unsupported on both baseline and fixed libraries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

All twelve original aliasing cases executed and passed in Delta run 36165204380 against merge ac916ff containing PR head 15fd9a3. The unchanged strict comparison reports zero regressions and passes after removing only these twelve stale entries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Use size_t for function-map indices with an explicit protobuf anchor conversion, and use Scala 2.13 collection converters in the Spark 4.1 metadata suite. Leave fixture paths and split-size assertions unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Select incompatible physical types by metadata field name rather than list position. Preserve fixture field order, row data, and existing native/fallback assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Document the whole-file split range, share the split-size threshold between assertions and configuration, and derive the fixture id column index from its schema. Preserve production and test behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Clarify the derived-schema branch with the file_name VARCHAR versus physical BIGINT collision and the reader initialization order. No behavior changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

felipepessoto · 2026-10-29T19:54:48Z

Thanks @rui-mo I rebased and CIs are all green. The previous error was a network issue downloading a package

github-actions Bot added CORE works for Gluten Core VELOX labels Sep 25, 2026

github-actions Bot added the INFRA label Sep 25, 2026

felipepessoto marked this pull request as ready for review September 26, 2026 01:45

Copilot AI lite review requested due to automatic review settings September 26, 2026 01:45

Copilot started reviewing on behalf of felipepessoto September 26, 2026 01:45

View session

Copilot AI reviewed Sep 26, 2026

View reviewed changes

Copilot AI lite review requested due to automatic review settings October 15, 2026 21:31

felipepessoto force-pushed the felipepessoto-metadata-schema-separation branch from 052015f to de5a5cd Compare October 15, 2026 21:31

Copilot started reviewing on behalf of felipepessoto October 15, 2026 21:55

View session

Copilot AI reviewed Oct 15, 2026

felipepessoto force-pushed the felipepessoto-metadata-schema-separation branch from de5a5cd to fd3a503 Compare October 19, 2026 20:45

Copilot AI lite review requested due to automatic review settings October 19, 2026 20:45

Copilot AI reviewed Oct 19, 2026

View reviewed changes

Copilot stopped reviewing on behalf of felipepessoto due to an error October 19, 2026 21:45
The job was not acquired by a runner even after multiple attempts.

Copilot AI lite review requested due to automatic review settings October 19, 2026 22:06

Copilot AI reviewed Oct 19, 2026

View reviewed changes

Comment thread cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc Resolved

Comment thread cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc Resolved

Comment thread cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc Resolved

Copilot stopped reviewing on behalf of felipepessoto due to an error October 19, 2026 22:34
The job was not acquired by a runner even after multiple attempts.

Copilot AI lite review requested due to automatic review settings October 22, 2026 20:41

felipepessoto force-pushed the felipepessoto-metadata-schema-separation branch from 3945786 to 824caed Compare October 22, 2026 20:41

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc Resolved

Comment thread cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc Resolved

Comment thread ...rc/test/scala/org/apache/spark/sql/execution/datasources/GlutenFileMetadataStructSuite.scala Resolved

Copilot AI lite review requested due to automatic review settings October 22, 2026 22:31

Copilot stopped reviewing on behalf of felipepessoto due to an error October 22, 2026 22:41
Workflow completed but conclusion is not available. This may indicate rate limiting or API issues.

Copilot AI reviewed Oct 22, 2026

Copilot stopped reviewing on behalf of felipepessoto due to an error October 23, 2026 00:12
The job was not acquired by a runner even after multiple attempts.

felipepessoto force-pushed the felipepessoto-metadata-schema-separation branch from 8e08819 to af06c58 Compare October 28, 2026 19:27

Copilot AI lite review requested due to automatic review settings October 28, 2026 19:27

felipepessoto changed the title ~~[VL] Keep synthetic scan metadata out of physical file schemas~~ [VL] Fix Parquet metadata scans with same-named data columns Oct 28, 2026

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Copilot stopped reviewing on behalf of felipepessoto due to an error October 28, 2026 20:03
The job was not acquired by a runner even after multiple attempts.

Copilot AI lite review requested due to automatic review settings October 28, 2026 20:25

Copilot started reviewing on behalf of felipepessoto October 28, 2026 20:44

View session

Copilot AI reviewed Oct 28, 2026

rui-mo approved these changes Oct 29, 2026

View reviewed changes

Comment thread cpp/velox/substrait/SubstraitToVeloxPlan.cc Resolved

Copilot AI lite review requested due to automatic review settings October 29, 2026 07:59

Copilot AI reviewed Oct 29, 2026

Copilot stopped reviewing on behalf of felipepessoto due to an error October 29, 2026 08:55
Workflow completed but conclusion is not available. This may indicate rate limiting or API issues.

felipepessoto and others added 9 commits October 29, 2026 10:32

[MINOR][VL] Explain scan metadata schema exclusion

8a4cb7e

Clarify the derived-schema branch with the file_name VARCHAR versus physical BIGINT collision and the reader initialization order. No behavior changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

felipepessoto force-pushed the felipepessoto-metadata-schema-separation branch from 1744c87 to 8a4cb7e Compare October 29, 2026 17:32

Copilot AI lite review requested due to automatic review settings October 29, 2026 17:32

Copilot AI reviewed Oct 29, 2026

Copilot stopped reviewing on behalf of felipepessoto due to an error October 29, 2026 18:45
The job was not acquired by a runner even after multiple attempts.

rui-mo merged commit 9801f41 into apache:main Oct 29, 2026
84 checks passed

felipepessoto deleted the felipepessoto-metadata-schema-separation branch October 29, 2026 20:35

Conversation

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

Uh oh!

What changes are proposed in this pull request?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

felipepessoto commented Sep 25, 2026

Uh oh!

github-actions Bot commented Sep 25, 2026

Uh oh!

felipepessoto commented Sep 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

felipepessoto commented Oct 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

felipepessoto commented Oct 28, 2026

Uh oh!

rui-mo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

felipepessoto commented Oct 29, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

felipepessoto commented Sep 25, 2026 •

edited

Loading