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

Reply via email to