fix: read Hive's null partition placeholder as NULL

[xanderbailey]

Which issue does this PR close?

• working towards ListingTable handling of missing partition values #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:

> 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 asdatafusion_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

[xanderbailey]

Big diff here sorry, but the actual change is very small

[github-actions]

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details

     Cloning apache/main
    Building datafusion v55.1.0 (current)
       Built [  60.409s] (current)
     Parsing datafusion v55.1.0 (current)
      Parsed [   0.033s] (current)
    Building datafusion v55.1.0 (baseline)
       Built [  60.123s] (baseline)
     Parsing datafusion v55.1.0 (baseline)
      Parsed [   0.034s] (baseline)
    Checking datafusion v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.697s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [ 123.064s] datafusion
    Building datafusion-catalog-listing v55.1.0 (current)
       Built [  41.155s] (current)
     Parsing datafusion-catalog-listing v55.1.0 (current)
      Parsed [   0.008s] (current)
    Building datafusion-catalog-listing v55.1.0 (baseline)
       Built [  40.285s] (baseline)
     Parsing datafusion-catalog-listing v55.1.0 (baseline)
      Parsed [   0.008s] (baseline)
    Checking datafusion-catalog-listing v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.096s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  82.631s] datafusion-catalog-listing
    Building datafusion-common v55.1.0 (current)
       Built [  34.508s] (current)
     Parsing datafusion-common v55.1.0 (current)
      Parsed [   0.068s] (current)
    Building datafusion-common v55.1.0 (baseline)
       Built [  34.395s] (baseline)
     Parsing datafusion-common v55.1.0 (baseline)
      Parsed [   0.069s] (baseline)
    Checking datafusion-common v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.707s] 229 checks: 228 pass, 1 fail, 0 warn, 31 skip

--- failure constructible_struct_adds_field: struct exhaustively constructible through public API adds field ---

Description:
A pub struct that could be exhaustively constructed with a literal using only public API has a new pub field, breaking existing exhaustive literals.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field ExecutionOptions.listing_table_hive_default_partition_name in /home/runner/work/datafusion/datafusion/datafusion/common/src/config.rs:902

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  70.788s] datafusion-common
    Building datafusion-datasource v55.1.0 (current)
       Built [  41.805s] (current)
     Parsing datafusion-datasource v55.1.0 (current)
      Parsed [   0.030s] (current)
    Building datafusion-datasource v55.1.0 (baseline)
       Built [  41.472s] (baseline)
     Parsing datafusion-datasource v55.1.0 (baseline)
      Parsed [   0.032s] (baseline)
    Checking datafusion-datasource v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.289s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  84.690s] datafusion-datasource
    Building datafusion-sqllogictest v55.1.0 (current)
       Built [  95.267s] (current)
     Parsing datafusion-sqllogictest v55.1.0 (current)
      Parsed [   0.016s] (current)
    Building datafusion-sqllogictest v55.1.0 (baseline)
       Built [  95.106s] (baseline)
     Parsing datafusion-sqllogictest v55.1.0 (baseline)
      Parsed [   0.017s] (baseline)
    Checking datafusion-sqllogictest v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.104s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [ 193.510s] datafusion-sqllogictest

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.28571% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.85%. Comparing base (0d79252) to head (f736cb0).
⚠️ Report is 36 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/common/src/config.rs	0.00%	7 Missing ⚠️
datafusion/datasource/src/statistics.rs	94.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #26310      +/-   ##
==========================================
+ Coverage   82.80%   82.85%   +0.05%     
==========================================
  Files        1149     1151       +2     
  Lines      454146   456142    +1996     
  Branches   454146   456142    +1996     
==========================================
+ Hits       376035   377932    +1897     
+ Misses      55260    55250      -10     
- Partials    22851    22960     +109     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[xanderbailey]

Hmm so if you specify the partition to be non-null now we will force it to be nullable... but the reverse was true before, if a user said it was non null, we would have made it nullable... This might need to be tracked as a follow-up. I'll have a look into this.

[xanderbailey]

Wonder if we need to be fancier here and follow what https://github.com/apache/hive/blob/f75c18b57ae4218db689d1e76fabb1343fd78973/common/src/java/org/apache/hadoop/hive/common/FileUtils.java#L280-L359 does?

[xanderbailey]

@andygrove I came across apache/datafusion-ballista#2580 which you opened recently and thought this might be of interest to you. Let me know if you think the approach here is reasonable. It was a little more difficult than I thought it might be.

[andygrove]

Heads up: this is an AI-assisted review pass. I've checked the points below against the diff, but I may have missed context or misjudged the intent, so please push back on anything that doesn't hold up.

Thanks for tackling this. Reading __HIVE_DEFAULT_PARTITION__ as NULL matches Hive and Spark, and the filter_partitioned_file change to treat a NULL predicate result as "no match" is a good catch.

Main concerns

1. Absent min/max for null partitions poisons merged statistics (datafusion/datasource/src/mod.rs, with_statistics)

A null partition value now gives min_value/max_value of Precision::Absent. Statistics::try_merge combines them with precision_min/precision_max, and Precision::min/max return Absent if either side is Absent. So a table with d=2024-01-01/ and d=__HIVE_DEFAULT_PARTITION__/ loses the min/max for d at the file-group and table level, even though every non-null file has exact bounds. Stats-based min(d)/max(d) and partition-bound pruning then silently degrade once any null partition directory exists. The slt tests wouldn't catch this because they fall back to a scan.

Could merging skip all-null containers when combining min/max? A test that merges stats from one null-partition file and one non-null file would pin this down.

2. MinMaxStatistics::new_from_files errors on a null partition (datafusion/datasource/src/statistics.rs)

with_statistics appends the partition column's stats to column_statistics, so get_min_max takes the first branch and calls .get_value().zip(...).ok_or_else(|| "statistics not found"). With Absent min/max, that now returns an error for any file with a null partition value. For a table with WITH ORDER (d) or a sort on a partition column, split_groups_by_statistics_with_target_partitions would then fail. If I'm reading this right, the failure is only logged at debug level and the split into non-overlapping ordered groups is skipped, so the plan gets extra sorts or loses its output ordering. Should a null partition be handled here as a null min/max (the row converter can sort nulls)?

3. distinct_count = Exact(0) (datafusion/datasource/src/mod.rs)

I'm not sure this is wrong, but I'd like to understand the intent. If distinct_count is meant to count NULL as a group, then Exact(0) under-reports and could make cardinality estimates for GROUP BY d or joins on d too low. If it excludes NULLs, 0 is consistent. Either way, when num_rows is Absent, null_count is Absent but distinct_count is still Exact(0), which claims more than is known. Would Absent be safer there?

4. Default-on behaviour change (datafusion/common/src/config.rs)

Any existing table with a literal col=__HIVE_DEFAULT_PARTITION__ directory, for example data written by something that used it as a real string value, will now read that value as NULL. WHERE col = '__HIVE_DEFAULT_PARTITION__' will then match nothing. Matching Hive/Spark by default is defensible, but it's a silent change of results, so it deserves an explicit call-out in the PR description and upgrade notes. The opt-out is also session-wide rather than per table (for example a ListingOptions field), so a table that mixes both conventions can't be read correctly. Is a session-wide setting enough for the use cases you have in mind?

5. All partition columns become nullable (datafusion/catalog-listing/src/table.rs)

try_new and scan now declare every partition column nullable, even when no null partition exists. That's why insert_to_external.slt changed. The side effects are that nullability-based optimizations (IS NOT NULL simplification, non-null join keys) are lost for every ListingTable. Other paths still build non-nullable partition fields, so a FileScanConfig built directly with a non-nullable partition field and a null partition value would hit Arrow's "declared non-nullable but contains null" error. Is it worth only making the field nullable when needed, or at least documenting this for direct FileScanConfig users?

Minor

• display_default (config.rs): it backticks any default containing _ or * across all config entries, including extension configs, to fix the rendering of one new default. Would it be narrower to quote just this default in its doc comment? As written it also changes how existing entries render, and | in a default still isn't escaped.

Overall the approach looks reasonable. Items 1 and 2 are the ones I'd want resolved before merge, since they turn into plan and statistics regressions as soon as a null partition exists.

[xanderbailey]

Thanks for the thorough review @andygrove! Going through the main points:

1. Absent min/max wipes out merged stats
Agreed, this is real. The obvious fix doesn't work, though. Giving a null partition Exact(<typed null>) min/max (the way AggregateExec represents all-null columns) would be worse than Absent. ScalarValue puts nulls below every value, so precision_min would pick the NULL, and min(d) answered from statistics would return NULL instead of 2024-01-01? Is that what we expect to see? Let me look a little more into this. But Absent is at least safe and given that before this PR a null partition on a non-string column couldn't be read at all, and the only case that loses stats is string partition columns, which used to return the literal placeholder (which I think is just a bug?) WDYT?

2. MinMaxStatistics::new_from_files errors on a null partition
Very nice catch, you were right, fixed in c6f7cd9.

3. distinct_count = Exact(0)
I think this one is correct as written but please challenge me. By convention distinct_count doesn't count NULLs anywhere in DataFusion: compute_group_ndv adds 1 separately when null_count > 0, and max_distinct_count in the join code documents that nulls don't count as distinct values. So Exact(0) doesn't under-report. It also stays exact when num_rows is Absent. The placeholder directory tells us every row is NULL, so there are zero distinct non-null values however many rows there are. null_count does depend on the row count, which is why it follows num_rows and becomes Absent. I also checked the main consumers: the GROUP BY estimate comes out at 1 group, the join estimate returns no estimate when the selectivity is 0, and filter/limit keep an exact 0.

4. Default-on behaviour change
Agreed it needs calling out. I'll add an entry to the 56.0.0 upgrade guide and update the PR description. On per-table versus session-wide: Hive's hive.exec.default.partition.name is itself a session/server setting, and Spark hardcodes __HIVE_DEFAULT_PARTITION__ with no option at all, so a session setting seems like a reasonable place to start to me?

5. All partition columns become nullable
I don't think we can make nullability conditional on the data. ListingTable::try_new builds the schema before any files are listed, and the listing can change between queries, so a schema that depends on what's on disk would be unstable. Spark and Hive also treat partition columns as nullable, so losing the IS NOT NULL simplification seems like it might be the right trade-off? Fair point about direct FileScanConfig users, though. pruned_partition_list is public and can now return null partition values, Is it okay to just document that callers building their own partition fields need them to be nullable?

[xanderbailey]

Going to write the update guide and then I want to dig into the first point a little more

[xanderbailey]

Dug into point 1 a bit more. I have managed to convince myself that this is okay from a correctness point of view as it currently stands, the proper fix is what you suggested: when merging min/max in Statistics::try_merge_iter, skip any input with an exact null_count == num_rows. Since it changes general stats merging for every source, I'd prefer to do it as a separate follow-up rather than grow this PR?

[xanderbailey]

Thanks again @andygrove, this should be good for another look now