gabotechs commented on code in PR #26593:
URL: https://github.com/apache/datafusion/pull/26593#discussion_r4470381761
##########
datafusion/common/src/partitioning.rs:
##########
@@ -89,10 +86,10 @@ pub fn validate_range_split_points(
split_points[0].values(),
split_points[1].values(),
sort_options,
- )? != Ordering::Less
+ )? == Ordering::Greater
Review Comment:
Making empty partitions irrepresentable in the `Partitioning::Range`
specification can be cornering.
For example, having an empty `FileGroup` in a parquet scan is something that
can happen, there's no invariant that prevent that, however, that'd
automatically make it incompatible with the current specification of
`Partitioning::Range`. At some point, `FileScanConfig` might want to expose
range partitioning here
https://github.com/apache/datafusion/blob/main/datafusion/datasource/src/file_scan_config/mod.rs#L908-L940,
so how should it react in the event of one of the `FileGroup`s being empty?
One other idea that comes to mind is to explicitly model empty partitions
somehow, maybe making the split points accept a `ScalarValue::Null` and
accounting for it?
Regardless of the mechanism, I do see value in making empty partitions
representable.
--
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]
