Datafusion: math function does not support array type f32

[lvheyang]

Describe the bug
Math function such as sqrt() / sin() ... does not support f32 type.

To Reproduce

Example Code

use std::sync::Arc;

use datafusion::arrow::array::{Float32Array, Float64Array};
use datafusion::arrow::datatypes::{DataType, Field, Schema};
use datafusion::arrow::record_batch::RecordBatch;
use datafusion::arrow::util::pretty;

use datafusion::datasource::MemTable;
use datafusion::error::Result;
use datafusion::prelude::*;

#[tokio::main]
async fn main() -> Result<()> {
    // define a schema.
    let schema = Arc::new(Schema::new(vec![
        Field::new("f64", DataType::Float64, false), 
        Field::new("f32", DataType::Float32, false),
    ]));

    // define data.
    let batch = RecordBatch::try_new(
        schema.clone(),
        vec![
            Arc::new(Float64Array::from(vec![1.0, 2.0, 3.0, 4.0, 5.0])),
            Arc::new(Float32Array::from(vec![1.0, 2.0, 3.0, 4.0, 5.0])),
        ],
    )?;

    let mut ctx = ExecutionContext::new();
    let provider = MemTable::try_new(schema, vec![vec![batch]])?;
    ctx.register_table("t", Arc::new(provider))?;

    // construct an expression corresponding to "SELECT sqrt(f32) FROM t" in SQL
    let df = ctx.sql("SELECT sqrt(f32) FROM t").unwrap(); // failed
    // let df = ctx.sql("SELECT sqrt(f64) FROM t").unwrap(); // success

    // execute
    let results = df.collect().await?;

    // print the results
    pretty::print_batches(&results)?;

    Ok(())
}

The code will panic, and the error output is

Error: ArrowError(InvalidArgumentError("column types must match schema types, expected Float64 but found Float32 at column index 0"))

Expected behavior

The code should not panic

Additional context

Possible reason:

in datafusion/src/physical_plan/functions.rs

The return type of math builtin functions is Float64

BuiltinScalarFunction::Abs
        | BuiltinScalarFunction::Acos
        | ...
        | BuiltinScalarFunction::Sqrt
        | BuiltinScalarFunction::Tan
        | BuiltinScalarFunction::Trunc => Ok(DataType::Float64),

But when compute the math function in datafusion/src/physical_plan/math_expressions.rs, when compute f32 array, it will return Float32Array

macro_rules! unary_primitive_array_op {
    ($VALUE:expr, $NAME:expr, $FUNC:ident) => {{
        match ($VALUE) {
            ColumnarValue::Array(array) => match array.data_type() {
                DataType::Float32 => {
                    let result = downcast_compute_op!(array, $NAME, $FUNC, Float32Array);
                    Ok(ColumnarValue::Array(result?))
                }
                DataType::Float64 => {
                    let result = downcast_compute_op!(array, $NAME, $FUNC, Float64Array);
                    Ok(ColumnarValue::Array(result?))
                }
                other => Err(DataFusionError::Internal(format!(
                    "Unsupported data type {:?} for function {}",
                    other, $NAME,
                ))),
            },
       ...

It seems that we should

1. Implicit cast f32 to float64 before we use math functions,
2. or when we infer schema we should also consider the input arguments' datatype. such as sqrt(f32) -> f32, sqrt(64) -> f64

I think the second is more reasonable

[lvheyang]

Supplement

Similar problems exist on abs()/signum() functions when dealing with i8/i16/i32. So I try to fix this problem using the second method in https://github.com/apache/arrow-datafusion/pull/716/files

But the unit test failed on datafusion\src\physical_plan\functions.rs:1865:9

---- physical_plan::functions::tests::test_functions stdout ----
thread 'physical_plan::functions::tests::test_functions' panicked at 'assertion failed: `(left == right)`
  left: `Float32`,
 right: `Float64`', datafusion\src\physical_plan\functions.rs:1865:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I checked it, found that case:

        test_function!(
            Exp,
            &[lit(ScalarValue::Float32(Some(1.0)))],
            Ok(Some((1.0_f32).exp() as f64)),
            f64,
            Float64,
            Float64Array
        );

I found that ScalarValue is different from ColumnarValue::Array, it was implicit cast to float64, in datafusion/src/physical_plan/math_expressions.rs

           ColumnarValue::Array(array) => match array.data_type() {
                DataType::Float32 => {
                    // did not cast to f64
                    let result = downcast_compute_op!(array, $NAME, $FUNC, Float32Array);  
                    Ok(ColumnarValue::Array(result?))
                }
               ...
            },
            ColumnarValue::Scalar(a) => match a {
                ScalarValue::Float32(a) => Ok(ColumnarValue::Scalar(
                    // cast to f64
                    ScalarValue::Float64(a.map(|x| x.$FUNC() as f64)),
                )),
                ...
            },

So is there anybody know or remember why there is such difference? And if or not the behavior is expected?

[lvheyang]

@alamb @houqp At your convenience, would you please help me review this issue?

[alamb]

@lvheyang I do not think there is any reason to implictly upcast from f32 --> f64 -- it is likely due to lack of testing. I am reviewing #716

[alamb]

But the unit test failed on datafusion\src\physical_plan\functions.rs:1865:9

So is there anybody know or remember why there is such difference? And if or not the behavior is expected?

I think the behavior of ColumnarValue::Scalar and ColumnarValue::Array(array) should be consistent with respect to any type coercion

[houqp]

Can confirm postgresql has the same generically typed behavior:

select
pg_typeof(sqrt(CAST('2' AS NUMERIC))),
pg_typeof(sqrt(CAST('2' AS DOUBLE PRECISION)));

Output:

pg_typeof	pg_typeof
numeric	double precision