Port aggregate test to sqllogictest

[jayzhan211]

Is your feature request related to a problem or challenge?

There are many rust test that exists in aggregate functions. Moving them to sqllogictest is easier to maintain and avoid additional testing functions like generic_test_op, assert_string_aggregate, assert_aggregate, assert_error_bounds, and so on. It also makes #8708 a lot easier.

10382 is an example of how tests are moved to.

I think most of them are Good first issue:

• [X] Sum #10382
• [X] Count #10383
• [X] average #10401
• [X] #10457
• [ ] min_max
• [ ] array_agg
• [ ] correlation
• [ ] median
• [ ] stddev
• [ ] variance

Describe the solution you'd like

Port rust test in datafusion/physical-expr/src/aggregate/tdigest.rs to datafusion/sqllogictest/test_files/aggregate.slt

We can just add the test to aggregate.slt even if there is a duplicated test case to make the review easier.

Describe alternatives you've considered

No response

Additional context

No response

[xinlifoobar]

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:

  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?

[jayzhan211]

I think we don't need to keep the test. The reason that you didn't get the error is that we have coercion already in building values. It is the expected result.