Skip to content

[branch-54] fix: read Hive's null partition placeholder as NULL (backport #26310) - #27308

Merged
alamb merged 2 commits into
apache:branch-54from
hareshkh:hareshkh/backport-26310-branch-54
Nov 12, 2026
Merged

alamb merged 2 commits into
apache:branch-54from
hareshkh:hareshkh/backport-26310-branch-54

Conversation

@hareshkh

@hareshkh hareshkh commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Hive writers use __HIVE_DEFAULT_PARTITION__ for NULL partition values. ListingTable currently 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 NULL consequently misses those rows.

What changes are included in this PR?

  • Read the Hive marker as a typed NULL and declare partition columns nullable.
  • Treat NULL partition-predicate results as non-matches and account for NULL partition values in file statistics.
  • Reject partitioned writes of the literal marker, which would otherwise read back as NULL.

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.

## 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)
@mbutrovich mbutrovich added the api change Changes the API exposed to users of the crate label Nov 10, 2026
@mbutrovich

Copy link
Copy Markdown
Contributor

I don't think this can go to branch-54 as written, because it is a breaking API change and a behavior change.

ExecutionOptions is generated by config_namespace!, which emits a plain pub struct that is not #[non_exhaustive] (config.rs#L133). Adding the public listing_table_hive_default_partition_name field breaks any downstream code that builds it with a struct literal, which the PR description already notes. That is a "Major Changes" item in the Cargo SemVer rules the API health policy refers to, and the target here is 54.2.0, a minor bump.

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 HIVE_DEFAULT_PARTITION_NAME constant, and the field.

It also changes behavior: all ListingTable partition fields become nullable, and partitioned writes now reject the marker string and NULL keys. The policy asks for a stated reason why such a change should ship in a patch release instead of the next major.

@hareshkh
hareshkh force-pushed the hareshkh/backport-26310-branch-54 branch from 5e8371f to f1c7958 Compare November 10, 2026 21:26
@hareshkh

Copy link
Copy Markdown
Contributor Author

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 ExecutionOptions compatibility break.

The rationale for the cherry-pick is that Hive NULL partitions currently fail the query for types such as dates, integers on branch-54 and they would now return NULL. However for string partitions, this technically changes behaviour - the literal __HIVE_DEFAULT_PARTITION__ becomes NULL, but this is matching its intended meaning in Hive partitioning.

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?

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hareshkh -- I think this is a clean cherry-pick with config removal so looks good to me

@alamb
alamb merged commit 31cb93f into apache:branch-54 Nov 12, 2026
10 of 13 checks passed
kamilk-pal pushed a commit to kamilk-pal/datafusion that referenced this pull request Nov 13, 2026
…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]>
alamb pushed a commit that referenced this pull request Nov 13, 2026
…) (#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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api change Changes the API exposed to users of the crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants