Repository navigation
[branch-54] fix: read Hive's null partition placeholder as NULL (backport #26310) - #27308
Conversation
## Which issue does this PR close? - working towards apache#18083 ## Rationale for this change Hive compliant writers put the rows whose partition value is null into a directory named `__HIVE_DEFAULT_PARTITION__` (Hive's `hive.exec.default.partition.name`). `ListingTable` casts that directory name straight to the partition column's type, so a table with a null partition cannot be queried at all unless the partition column happens to be a string: ```sql > create external table t (id int, d date) stored as parquet partitioned by (d) location 'data/'; > select * from t; Arrow error: Cast error: Cannot cast string '__HIVE_DEFAULT_PARTITION__' to value of Date32 type ``` For a string partition column the query runs, but the value comes back as the literal `__HIVE_DEFAULT_PARTITION__` rather than NULL, and `WHERE d IS NULL` never matches it because partition columns are declared non-nullable. Hive-written tables are a common input, and both Spark and Impala read these values as NULL. We are not touching writes in this PR - just reads. We also haven’t fixed the /date=/a.parquet problem here either. ## What changes are included in this PR? 1. A partition value equal to the placeholder is read as a typed NULL rather than parsed (`helpers::try_into_partitioned_file`). 2. `ListingTable` declares partition columns nullable, in both the table schema and the file source's `TableSchema`, so the NULL can be represented and `IS NULL` selects the null partition. 3. A new option, `datafusion.execution.listing_table_hive_default_partition_name`, defaulting to `__HIVE_DEFAULT_PARTITION__`, so a table written with a different `hive.exec.default.partition.name` can be read. The empty name turns the mapping off and reads the directory name as the value. The default is also exposed as`datafusion_common::config::HIVE_DEFAULT_PARTITION_NAME`. 4. `filter_partitioned_file` no longer treats a NULL predicate result as a match. That is reachable now that a partition value can be NULL 5. Claude caught this one not me: In the generated `configs.md`, defaults containing `_` or `*` are shown as code. Markdown would otherwise read `__HIVE_DEFAULT_PARTITION__` as emphasis and render it as bold text; this also backticks the two existing such defaults, `nulls_max` and `start_offset`. ## What is the testing strategy for this PR? Unit tests and slt tests added. ## Are there any user-facing changes? partition columns are now forced to be nullable listing_table_hive_default_partition_name added (cherry picked from commit 407bef6)
|
I don't think this can go to
The release management policy says patch releases carry fixes only, and lists new config options, new public APIs, and breaking API changes as not recommended for backport. This PR adds the config option, the public It also changes behavior: all |
5e8371f to
f1c7958
Compare
|
Sorry @mbutrovich, I was still iterating on this PR and should have opened it as a draft. Thanks for the prompt review. I’ve removed the execution option and public constant to avoid the The rationale for the cherry-pick is that Hive NULL partitions currently fail the query for types such as dates, integers on Partition columns also become nullable, and the rejection of writing the reserved marker as a literal partition value remains. Defer to you on if this appropriate for branch-54. Could you please take another look? |
…he#26310) Backport of apache#26310 to branch-55, based on the branch-54 backport (apache#27308). Read the Hive `__HIVE_DEFAULT_PARTITION__` marker as a typed NULL, declare partition columns nullable, treat NULL partition-predicate results as non-matches, account for NULL partition values in file statistics, and reject partitioned writes of the literal marker. Like the branch-54 backport, this uses a fixed marker without the new `listing_table_hive_default_partition_name` option or public constant. (cherry picked from commit 31cb93f) Co-Authored-By: Claude Opus 5.5 <[email protected]>
…) (#27392) ## Which issue does this PR close? - Backport of #26310 to `branch-55` - Part of #26868 --- ## Differences from the branch-54 backport (#27308) This backport is the same as #27308. Comparing the two diffs with `git range-diff` shows only these two changed lines that differ: 1. **`table.rs`: indentation only.** The same `false` → `true` nullability change is one level deeper, because branch-55 builds the schema with `TableSchemaBuilder` instead of `TableSchema::new`. - branch-55: [table.rs#L347-L348](https://github.com/kamilk-pal/datafusion/blob/a14dd16c2fa438d6ef90631b7e118383c2c916c5/datafusion/catalog-listing/src/table.rs#L347-L348) - branch-54: [table.rs#L345-L346](https://github.com/apache/datafusion/blob/31cb93f88404daa7d64048aaec7fa802aaaa0079/datafusion/catalog-listing/src/table.rs#L345-L346) 2. **`listing_table_partitions.slt`: comment only.** It says "branch-55" instead of "branch-54". branch-55 keeps the explicit sort just like branch-54, so the expected plan is unchanged. - branch-55: [listing_table_partitions.slt#L217](https://github.com/kamilk-pal/datafusion/blob/a14dd16c2fa438d6ef90631b7e118383c2c916c5/datafusion/sqllogictest/test_files/listing_table_partitions.slt#L217) - branch-54: [listing_table_partitions.slt#L217](https://github.com/apache/datafusion/blob/31cb93f88404daa7d64048aaec7fa802aaaa0079/datafusion/sqllogictest/test_files/listing_table_partitions.slt#L217) All other added and removed lines are identical in both backports. Some surrounding code differs because the base branches differ (for example, branch-55 percent-decodes partition values), but neither backport changes it. Co-authored-by: Haresh Khanna <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
Which issue does this PR close?
branch-5454.2.0(minor/patch) Release #23805Rationale for this change
Hive writers use
__HIVE_DEFAULT_PARTITION__for NULL partition values.ListingTablecurrently parses this marker as the partition column's type, causing cast errors for non-string columns and returning the literal marker for string columns.IS NULLconsequently misses those rows.What changes are included in this PR?
This backport uses a fixed marker, without the new execution option or public constant. Ordering tests reflect branch-54's existing explicit-sort fallback.
Are these changes tested?
Yes
Are there any user-facing changes?
Yes. Hive marker directories yield NULL values, and partition columns are nullable. Writing the literal marker returns an error. The marker cannot be configured or disabled in this backport.
AI assistance: Codex.