You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "ShayanGho (via GitHub)" <gi...@apache.org> on 2026/09/12 02:42:08 UTC
[PR] fix: drop partition columns already present in the file schema of a ListingTable [datafusion]
ShayanGho opened a new pull request, #25212:
URL: https://github.com/apache/datafusion/pull/25212
## Which issue does this PR close?
- Closes #17420.
## Rationale for this change
Parquet files written with `datafusion.execution.keep_partition_by_columns = true` physically contain the partition column. Reading them back as a hive-partitioned table then fails:
```sql
SET datafusion.execution.keep_partition_by_columns = TRUE;
COPY (SELECT 1 AS id, 'a' AS grp) TO '/tmp/out' STORED AS PARQUET PARTITIONED BY (grp);
CREATE EXTERNAL TABLE t2 STORED AS PARQUET PARTITIONED BY (grp) LOCATION '/tmp/out';
SELECT * FROM t2;
-- Schema error: Schema contains duplicate qualified field name t2.grp
```
`DESCRIBE t2` shows `grp` twice: once with the type inferred from the files and once with the partition column type. The same error is hit without `PARTITIONED BY` (the factory infers hive partitions from the directory names by default) and when querying the directory path directly (`SELECT * FROM '/tmp/out/'`).
`ListingTable::try_new` unconditionally appends every configured partition column to the inferred file schema. When the file schema already has a field with that name, the table schema ends up with a duplicate. `CREATE EXTERNAL TABLE` with an explicit column list already works because the factory projects the partition columns out of the provided schema; the inferred-schema path had no equivalent.
## What changes are included in this PR?
`ListingTable::try_new` now drops any file-schema field whose name matches a configured partition column before appending the partition columns. The partition value continues to be read from the path and keeps the declared (or inferred) partition column type, matching the existing behaviour of the explicit-schema path. The trimmed schema is also what `create_file_source` and the file-schema fingerprint use, so the physical scan and the table schema agree.
## What is the testing strategy for this PR?
- New unit test `test_partition_column_present_in_file_schema_is_not_duplicated` in `datafusion/core/src/datasource/listing/table.rs` builds a `ListingTable` whose file schema already contains the partition column and asserts the table schema lists it once, with the partition type. It failed before the fix with `["a", "pid", "pid"]`.
- Two new cases in `datafusion/sqllogictest/test_files/copy.slt` reuse the existing `keep_partition_by_columns` output directory and create a table over the whole directory, once with an explicit `PARTITIONED BY` and once relying on partition inference. Both failed before the fix with the duplicate field error.
- The existing `parquet_overlapping_columns` test in `datafusion/core/tests/sql/path_partition.rs` asserted that a partition column sharing a name with a file column must raise an error. This PR intentionally changes that behaviour to match how `CREATE EXTERNAL TABLE` with an explicit column list already handles the overlap. The test now asserts the new behaviour: the column appears once, with the partition type, and its values come from the path.
## Are there any user-facing changes?
Reading hive-partitioned Parquet data whose files also contain the partition column now works instead of raising a schema error.
Conflict policy, made explicit: when a file column and a partition column share a name, the partition column's path-derived value and declared type take precedence. The file column is ignored, and its values are not validated against the path value.
No API changes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: drop partition columns already present in the file schema of a ListingTable [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #25212:
URL: https://github.com/apache/datafusion/pull/25212#discussion_r4082401275
##########
datafusion/catalog-listing/src/table.rs:
##########
@@ -225,6 +225,32 @@ impl ListingTable {
.options
.ok_or_else(|| internal_datafusion_err!("No ListingOptions provided"))?;
+ // Files may physically contain the partition columns, for example when
+ // they were written with `keep_partition_by_columns = true`. Partition
+ // column values are always taken from the path, so drop such columns
+ // from the file schema to avoid duplicated fields in the table schema.
+ let file_schema = if options
+ .table_partition_cols
+ .iter()
+ .any(|(name, _)| file_schema.field_with_name(name).is_ok())
+ {
+ let indices: Vec<usize> = file_schema
+ .fields()
+ .iter()
+ .enumerate()
+ .filter(|(_, field)| {
+ !options
Review Comment:
Could we add a small unit case with two overlapping partition columns, with one of them appearing in the middle of the physical schema? It would be useful to assert that the final order has all remaining file fields first, followed by the partition fields in their configured order. The current single trailing-column case covers deduplication well, but does not exercise the projection and ordering behavior introduced by this filter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: drop partition columns already present in the file schema of a ListingTable [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25212:
URL: https://github.com/apache/datafusion/pull/25212#issuecomment-5643053100
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25212?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `90.47619%` with `4 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.93%. Comparing base ([`f8cc678`](https://app.codecov.io/gh/apache/datafusion/commit/f8cc678272c50c781a56b74b49912b999aebd89e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b4e7b2c`](https://app.codecov.io/gh/apache/datafusion/commit/b4e7b2cb0e8b48fc6bf0a8866133098933d4ade5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/25212?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/core/src/datasource/listing/table.rs](https://app.codecov.io/gh/apache/datafusion/pull/25212?src=pr&el=tree&filepath=datafusion%2Fcore%2Fsrc%2Fdatasource%2Flisting%2Ftable.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9kYXRhc291cmNlL2xpc3RpbmcvdGFibGUucnM=) | 87.50% | [0 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/catalog-listing/src/table.rs](https://app.codecov.io/gh/apache/datafusion/pull/25212?src=pr&el=tree&filepath=datafusion%2Fcatalog-listing%2Fsrc%2Ftable.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9jYXRhbG9nLWxpc3Rpbmcvc3JjL3RhYmxlLnJz) | 94.44% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #25212 +/- ##
==========================================
- Coverage 81.93% 81.93% -0.01%
==========================================
Files 1133 1133
Lines 423529 423571 +42
Branches 423529 423571 +42
==========================================
+ Hits 347028 347048 +20
- Misses 55910 55923 +13
- Partials 20591 20600 +9
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25212?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: drop partition columns already present in the file schema of a ListingTable [datafusion]
Posted by "ShayanGho (via GitHub)" <gi...@apache.org>.
ShayanGho commented on PR #25212:
URL: https://github.com/apache/datafusion/pull/25212#issuecomment-5705771396
Hello @alamb, when you get a chance can you please review this PR?
it closes #17420 (schema error reading partitioned Parquet written with keep_partition_by_columns). One behavioral change worth checking, when a file column and a partition column share a name, I chose path-derived partition column's path-derived value
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: drop partition columns already present in the file schema of a ListingTable [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #25212:
URL: https://github.com/apache/datafusion/pull/25212
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: drop partition columns already present in the file schema of a ListingTable [datafusion]
Posted by "ShayanGho (via GitHub)" <gi...@apache.org>.
ShayanGho commented on code in PR #25212:
URL: https://github.com/apache/datafusion/pull/25212#discussion_r4088897745
##########
datafusion/catalog-listing/src/table.rs:
##########
@@ -225,6 +225,32 @@ impl ListingTable {
.options
.ok_or_else(|| internal_datafusion_err!("No ListingOptions provided"))?;
+ // Files may physically contain the partition columns, for example when
+ // they were written with `keep_partition_by_columns = true`. Partition
+ // column values are always taken from the path, so drop such columns
+ // from the file schema to avoid duplicated fields in the table schema.
+ let file_schema = if options
+ .table_partition_cols
+ .iter()
+ .any(|(name, _)| file_schema.field_with_name(name).is_ok())
+ {
+ let indices: Vec<usize> = file_schema
+ .fields()
+ .iter()
+ .enumerate()
+ .filter(|(_, field)| {
+ !options
Review Comment:
Thank you, added a test for two overlapping partition columns with one mid-schema.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org