gene-bordegaray commented on code in PR #23231:
URL: https://github.com/apache/datafusion/pull/23231#discussion_r3537352457
##########
datafusion/physical-plan/src/repartition/mod.rs:
##########
@@ -2241,6 +2533,124 @@ mod tests {
Arc::new(Schema::new(vec![Field::new("c0", DataType::UInt32, false)]))
}
+ fn nullable_u32_schema() -> Arc<Schema> {
Review Comment:
Thanks for adding the coverag.
One thing on the test helpers. I think they may be a bit more than needed.
The important cases to cover are:
- single-key and compund-key routing
- ASC / DESC behavior
- null ordering
- non-primitive/string routing
For the compound-key test, I don’t think it needs to use `Int32` and this
causes us to have an extra helper here. I think we can just switch that guy to
`UInt32` and eliminate helpers.
I think we can also simplifying the `u32` collectors into one nullable-aware
helper and either dropping `assert_partition_contains_only_u32_values` or
making the assertions more direct.
--
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]
