Convert scalar value to correct type based on arrow data type.

[viirya]

Description

The description of the main changes of your pull request

This is a follow up of #327 for the comment #327 (comment).
For the max/min column scalar values, we convert it to correct ScarlarValue based on arrow datatype.

Related Issue(s)

Documentation

[viirya]

I tried to add a test on date type with delta-0.8.0-date table. But the query like SELECT max(date), min(date) FROM date_table throws an error:

Error: Internal("Unsupported CAST from Date32 to Utf8")

Seems the aggregation on date types is not ready?

I can look at it later.

[houqp]

Seems the aggregation on date types is not ready?

Yes, datafusion only supports string, numeric and timestamp types for min max aggregation at the moment. We need to update physical_plan/expressions/min_max.rs and physical_plan/aggregates.rs to add date support.

[houqp]

Is there any reason why we prefer downcasting from i64 to i32 over direct i32::try_from(value) call?

[viirya]

i32::try_from call only applies on an input of ScalarValue::Int32. For number scalar value, we all use ScalarValue::Int64 to compare to get max/min values. That's said here the input is ScalarValue::Int64 and i32::try_from will throw Cannot convert ... error.

[houqp]

ha, i see. perhaps in the future, we could change to_scalar_value to do the scalar value type cast based on field type so we can avoid this intermediate 64 bits conversion overhead.

[houqp]

FYI @Dandandan

[viirya]

Seems the aggregation on date types is not ready?

Yes, datafusion only supports string, numeric and timestamp types for min max aggregation at the moment. We need to update physical_plan/expressions/min_max.rs and physical_plan/aggregates.rs to add date support.

Thanks for the info. I may take some time looking at it later.

[viirya]

@houqp @Dandandan Thanks for the review. Is this good to go?