comphead commented on code in PR #7528:
URL: https://github.com/apache/datafusion-comet/pull/7528#discussion_r4532981269
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -520,6 +520,16 @@ macro_rules! cast_float_to_int32_up {
}};
}
+/// 10^scale, or an error for a scale outside 0..=38. Negative scales need
+/// spark.sql.legacy.allowNegativeScaleOfDecimal and can still arrive from the
Parquet schema
+/// adapter.
Review Comment:
Which Parquet schema adapter path can still deliver a negative scale here?
If a read schema such as `DECIMAL(10,-2)` (with
`spark.sql.legacy.allowNegativeScaleOfDecimal=true`) over an INT32 or decimal
column reaches `cast_int_to_decimal128` or the decimal to integral kernels
through `replace_with_spark_cast`, then a real query now fails with
`INTERNAL_ERROR` instead of returning zeros. That is better than wrong results,
but the scan should then fall back for that schema, and a test for it would be
worth adding. If no current path gets here, could the comment say that, as
#7022 does, so it does not read as a known reachable case?
--
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]
