xinlifoobar commented on issue #10384: URL: https://github.com/apache/datafusion/issues/10384#issuecomment-2112041649
Hello @jayzhan211 , I am trying to ramp up the repo via porting the min_max tests to sqllogictests. During the migrations, however, I found there are inconsistent behavours. An example is error handling inside rust class versus through the SQL engine as such: - Before: ```rs let left = ScalarValue::Decimal128(Some(123), 10, 2); let right = ScalarValue::Decimal128(Some(124), 10, 2); let result = max(&left, &right)?; assert_eq!(result, right); let right = ScalarValue::Decimal128(Some(124), 10, 3); let result = max(&left, &right); let err_msg = format!( "MIN/MAX is not expected to receive scalars of incompatible types {:?}", (Decimal128(Some(123), 10, 2), Decimal128(Some(124), 10, 3)) ); let expect = DataFusionError::Internal(err_msg); assert!(expect .strip_backtrace() .starts_with(&result.unwrap_err().strip_backtrace())); ``` - After: ``` statement ok DROP TABLE decimals; statement ok CREATE TABLE decimals_error (value DECIMAL(10, 2)); statement ok INSERT INTO decimals_error VALUES (123.00), (arrow_cast(124.00, 'Decimal128(10, 3)')); query R SELECT MAX(value) FROM decimals_error ---- 124 ``` As far as I could tell, the error will be lost permanently. What's your opinions on such migrations? Shall we keep the tests that are different still in the rs files? -- 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