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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to