alamb commented on code in PR #25227:
URL: https://github.com/apache/datafusion/pull/25227#discussion_r4050838674
##########
datafusion/physical-expr/src/expressions/cast.rs:
##########
@@ -260,6 +263,8 @@ impl CastExpr {
(Int8, Int16 | Int32 | Int64)
| (Int16, Int32 | Int64)
| (Int32, Int64)
+ | (Int32, Date32)
Review Comment:
these are lossless because they have the same underlying representation?
Should we also have entries for Date64 and Int64?
##########
datafusion/physical-expr/src/projection.rs:
##########
@@ -872,15 +872,39 @@ fn project_column_statistics_through_expr(
return inner_stats;
}
+ let min_value = inner_stats
+ .min_value
+ .cast_to(target_type)
+ .unwrap_or(Precision::Absent);
+ let max_value = inner_stats
+ .max_value
+ .cast_to(target_type)
+ .unwrap_or(Precision::Absent);
+ let source_type = inner_stats
+ .min_value
+ .get_value()
+ .or_else(|| inner_stats.max_value.get_value())
+ .map(ScalarValue::data_type);
+ // Copy extrema only for casts that preserve order and cannot discard
values
+ // or fail within the input domain. Copying string endpoints into a numeric
+ // domain, for example, does not bound the converted column. Merely casting
+ // a failing endpoint to NULL also cannot establish the remaining extrema.
+ let preserves_values = source_type.is_some_and(|source_type| {
+ CastExpr::check_bigger_cast(target_type, &source_type)
+ || (source_type.is_integer() && target_type.is_integer()
Review Comment:
I think this would be easier to read if we also factored this specific
integer check into its own function too so the logic isn't quite as complex
Something like
```rust
CastExpr::check_bigger_cast(target_type, &source_type)
|| is_within_extrema(&inner_stats.min_value,
&inner_stats.max_value, &min_value, &max_value)
```
Then you could also add some examples / unit tests for the extrema check
##########
datafusion/sqllogictest/test_files/parquet_statistics.slt:
##########
@@ -186,6 +186,39 @@ physical_plan
statement ok
DROP TABLE typed_table;
+######
Review Comment:
can we also write a test to check the extrema condition (like casting an
int64 in parquet into an int8)?
--
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]
