Potentially Overly-Stringent Type Checking for array_position

Describe the bug

This query: SELECT array_position(a, 1) FROM test will fail if a is a list of int32s, due to Error: Plan("array_position received incompatible types: '[Int32, Int64]'.").

I understand that the value to find couldn't be found if 1 was actually a number bigger than an i32 could hold, but it seems to me this should try to work, then fail in that case.

To Reproduce

This script should repro:

use std::sync::Arc;

use datafusion::{
    arrow::{
        array::{Array, Int32Builder, ListBuilder},
        datatypes::{Field, Fields, Schema},
    },
    datasource::MemTable,
    execution::context::SessionContext,
};

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let mut list_builder = ListBuilder::new(Int32Builder::new());

    list_builder.append_value([Some(1), Some(2), Some(3)]);
    list_builder.append(true);

    let list_array = list_builder.finish();

    let data_type = list_array.data_type();

    let fields = Fields::from(vec![Field::new("a", data_type.clone(), true)]);
    let schema = Arc::new(Schema::new(fields));

    let record_batch = datafusion::arrow::record_batch::RecordBatch::try_new(
        schema.clone(),
        vec![Arc::new(list_array)],
    )?;

    let table = MemTable::try_new(schema, vec![vec![record_batch]])?;

    let ctx = SessionContext::new();
    ctx.register_table("test", Arc::new(table))?;

    let sql = "SELECT array_position(a, 1) FROM test";

    let plan = ctx.sql(sql).await?.collect().await?;

    Ok(())
}

Expected behavior

I'd expect array_position to give a best effort to find the query value and fail if the passed value is incompatible due to its magnitude not immediately by its type.

Additional context

No response

@tshauck

Thank you, @tshauck, for your report. You have made a nice catch. The issue is caused by:

fn check_datatypes(name: &str, args: &[&ArrayRef]) -> Result<()> {
    let data_type = args[0].data_type();
    if !args.iter().all(|arg| {
        arg.data_type().equals_datatype(data_type)
            || arg.data_type().equals_datatype(&DataType::Null)
    }) {
        let types = args.iter().map(|arg| arg.data_type()).collect::<Vec<_>>();
        return plan_err!("{name} received incompatible types: '{types:?}'.");
    }

    Ok(())
}

It does not consider similar types with different magnitudes.

And DuckDB can accept

D SELECT list_position([1, 2], cast(1 as SMALLINT));
┌───────────────────────────────────────────────────────────┐
│ list_position(main.list_value(1, 2), CAST(1 AS SMALLINT)) │
│                           int32                           │
├───────────────────────────────────────────────────────────┤
│                                                         1 │
└───────────────────────────────────────────────────────────┘

D SELECT list_position([1, 2], cast(1 as FLOAT8));
┌─────────────────────────────────────────────────────────┐
│ list_position(main.list_value(1, 2), CAST(1 AS DOUBLE)) │
│                          int32                          │
├─────────────────────────────────────────────────────────┤
│                                                       1 │
└─────────────────────────────────────────────────────────┘

@Weijun-H

@Weijun-H I think in this case we should convert int32 list to int64 list.
If we got inconsistent types for array_position, we should try type coercion first, if fails, it should be an error.
I'm not sure how duckdb handle this internally. But, they have the same behavior if you convert them to consistent types.

select list_position([1.1, 3.0], CAST(3 as INT)); convert CAST(3 as INT) into 3.0
select list_position([1, 3], 3.0); convert [1,3] to [1.0, 3.0]
SELECT list_position([1, 2], cast(1 as FLOAT8)); convert [1, 2] to [1.0, 2.0]

Probably changing

            BuiltinScalarFunction::ArrayPosition => {
                Signature::variadic_any(self.volatility())
            }

to ArrayAndElement fix this issue

Yes, I think so. We could extend ArrayAndElement after #5340

Navigation Menu

Repository navigation

Potentially Overly-Stringent Type Checking for `array_position` #9093

Describe the bug

To Reproduce

Expected behavior

Additional context

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Potentially Overly-Stringent Type Checking for array_position #9093

Description

Describe the bug

To Reproduce

Expected behavior

Additional context

Activity

Weijun-H commented on Feb 1, 2024

jayzhan211 commented on Feb 1, 2024

Weijun-H commented on Feb 1, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Potentially Overly-Stringent Type Checking for `array_position` #9093