mbutrovich commented on code in PR #26310:
URL: https://github.com/apache/datafusion/pull/26310#discussion_r4482770280
##########
datafusion/sqllogictest/test_files/listing_table_partitions.slt:
##########
@@ -127,3 +127,118 @@ select year, count(*) from root_file_test group by year
order by year;
----
2024 2
2025 1
+
+# Test: a null partition value, which Hive compliant writers spell with a
+# `__HIVE_DEFAULT_PARTITION__` directory, is read as NULL.
+
+query I
+copy (values(1, 'alice'))
+to
'test_files/scratch/listing_table_partitions/null_part/d=2024-01-01/data.parquet';
+----
+1
+
+query I
+copy (values(2, 'bob'))
+to
'test_files/scratch/listing_table_partitions/null_part/d=__HIVE_DEFAULT_PARTITION__/data.parquet';
+----
+1
+
+statement count 0
+create external table null_part (column1 int, column2 varchar, d date)
+stored as parquet
+partitioned by (d)
+location 'test_files/scratch/listing_table_partitions/null_part/';
+
+query ITD
+select column1, column2, d from null_part order by column1;
+----
+1 alice 2024-01-01
+2 bob NULL
+
+query ITD
+select column1, column2, d from null_part where d is null;
+----
+2 bob NULL
+
+# The null partition does not match an equality filter.
+query ITD
+select column1, column2, d from null_part where d = '2024-01-01';
+----
+1 alice 2024-01-01
+
+# ... nor an inequality filter.
+query ITD
+select column1, column2, d from null_part where d <> '2024-01-01';
+----
+
+query ITD
+select column1, column2, d from null_part where d not in ('2024-01-01');
+----
+
+# Statistics for the null partition count its rows as nulls.
+query I
+select count(d) from null_part;
+----
+1
+
+query DD
+select min(d), max(d) from null_part;
+----
+2024-01-01 2024-01-01
Review Comment:
Could we add a case for a table that infers its partition columns? Without a
declared schema, `CREATE EXTERNAL TABLE` infers `d` as `Dictionary(UInt16,
Utf8)`, which is the usual way to read a Hive-written table. The new tests only
reach `ScalarValue::try_new_null` with `Date32`, `Int32`, and `Utf8`. Something
like this, after the `min`/`max` query:
```
# Partition columns inferred from the directories are dictionary encoded.
statement count 0
create external table null_part_inferred
stored as parquet
location 'test_files/scratch/listing_table_partitions/null_part/';
query TT
select arrow_typeof(d), d from null_part_inferred where d is null;
----
Dictionary(UInt16, Utf8) NULL
query I
select column1 from null_part_inferred where d <> '2024-01-01';
----
```
##########
datafusion/datasource/src/file_groups.rs:
##########
@@ -360,7 +360,7 @@ impl FileGroupPartitioner {
/// Partition column statistics are derived from the individual file partition
values:
/// - `min` = minimum partition value across all files in the group
/// - `max` = maximum partition value across all files in the group
Review Comment:
Could we update the `min` and `max` lines too, now that this block covers
null partitions? A file with a null partition value gets absent `min`/`max`
from
[`with_statistics`](https://github.com/apache/datafusion/blob/bdc97394c1c0ce26fa0a29ccab954503cb5d4b70/datafusion/datasource/src/mod.rs#L347-L355).
When the group's statistics are merged,
[`precision_min`](https://github.com/apache/datafusion/blob/b6b2e0e5d5f119f46b7fb6288827aee7041ab00d/datafusion/common/src/stats.rs#L887-L910)
returns `Precision::Absent` if either side is absent. So a group with one null
partition has no bounds for that column, and these two lines say it does.
#26748 tracks the merge fix, and the doc can point there until it lands.
```suggestion
/// - `min` = minimum partition value across all files in the group, or
absent if any
/// file in the group has a null partition value (see #26748)
/// - `max` = maximum partition value across all files in the group, or
absent if any
/// file in the group has a null partition 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: [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]
