kosiew commented on code in PR #23226:
URL: https://github.com/apache/datafusion/pull/23226#discussion_r3541085574
##########
datafusion/catalog-listing/src/helpers.rs:
##########
@@ -272,7 +277,8 @@ pub fn evaluate_partition_prefix<'a>(
Some(PartitionValue::Single(val)) => {
// if a partition only has a single literal value, then it can
be added to the
// prefix
- parts.push(format!("{p}={val}"));
+ let encoded = encode_partition_value(val);
+ parts.push(format!("{p}={encoded}"));
Review Comment:
I think this still needs a tweak. `evaluate_partition_prefix` now always
percent-encodes filtered partition values before building the listing prefix.
That fixes encoded paths like `category=Electronics%2FComputers`, but it makes
prefix pruning non-conservative for valid unencoded paths.
For example, a table may contain `category=John Doe/data.parquet`.
DataFusion's hive writer currently builds partition directories with raw
`format!("{}={}", name, value)`, and `parse_partitions_for_path` would parse
that value as `John Doe`. So `WHERE category = 'John Doe'` should still match
it.
With this change, prefix pruning lists only `category=John%20Doe`, so the
raw `category=John Doe` directory is never seen and the query can silently miss
rows.
Since listing prefix pruning is an optimization, it needs to stay
conservative. For values where `encode_partition_value(val) != val`, I think we
should either disable exact prefix pruning by returning `None` or stopping at
that partition, or otherwise make sure both raw and percent-encoded spellings
are listed before filtering.
--
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]
