2010YOUY01 commented on code in PR #16831: URL: https://github.com/apache/datafusion/pull/16831#discussion_r2218150407
########## datafusion/common/src/scalar/mod.rs: ########## @@ -1382,6 +1382,12 @@ impl ScalarValue { DataType::Float16 => ScalarValue::Float16(Some(f16::from_f32(1.0))), DataType::Float32 => ScalarValue::Float32(Some(1.0)), DataType::Float64 => ScalarValue::Float64(Some(1.0)), + DataType::Decimal128(precision, scale) => { + ScalarValue::Decimal128(Some(1), *precision, *scale) Review Comment: If we create a `new_one()` for type `Decimal128(3,3)`, the result in the natural scale will be 0.001: https://github.com/apache/datafusion/blob/386985788a35e474b829d46c9dccdfd7c5117d98/datafusion/sql/src/expr/value.rs#L467 I think this function is supposed to construct 1 in the natural scale? So in this example it should be converted to `Decimal128(Some(1000), 3, 3)`? ########## datafusion/optimizer/src/simplify_expressions/utils.rs: ########## @@ -17,54 +17,14 @@ //! Utility functions for expression simplification +use arrow::datatypes::i256; use datafusion_common::{internal_err, Result, ScalarValue}; use datafusion_expr::{ expr::{Between, BinaryExpr, InList}, expr_fn::{and, bitwise_and, bitwise_or, or}, Expr, Like, Operator, }; -pub static POWS_OF_TEN: [i128; 38] = [ Review Comment: I guess this lookup table is used for performance? We can do some measurements to check if it's useful. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org