sundy-li / databend

FuseQuery is a Distributed SQL Query Engine at scale
Other
2 stars 0 forks source link

bug: nested try_cast in having cause panic #85

Open sundy-li opened 2 months ago

sundy-li commented 2 months ago

Search before asking

Version

v1.2.314-nightly

What's Wrong?

nested try_cast in having cause panic

panicked at src/query/expression/src/evaluator.rs:593:58:
called `Option::unwrap()` on a `None` value

How to Reproduce?

mysql> create table test(a int not null, b timestamp not null);
Query OK, 0 rows affected (0.13 sec)

mysql> insert into test values(1, '2024-01-01');
Query OK, 1 row affected (0.10 sec)

mysql> select * from test having try_cast(b as boolean);
ERROR 1105 (HY000): BadArguments. Code: 1006, Text = error:
  --> SQL:1:36
  |
1 | select * from test having try_cast(b as boolean)
  |                                    ^ unable to cast type `Timestamp` to type `Boolean NULL`

mysql> select * from test having try_cast(try_cast(b as timestamp null) as boolean);
ERROR 1105 (HY000): StorageOther. Code: 4000, Text = segment pruning failure, task 47868 panicked.

Are you willing to submit PR?

dosubot[bot] commented 2 months ago

To fix the panic caused by nested try_cast in the HAVING clause in version v1.2.314-nightly, you need to ensure that the run_try_cast function handles nested try_cast operations correctly. Here is a complete and corrected implementation:

pub fn run_try_cast(
    &self,
    span: Span,
    src_type: &DataType,
    dest_type: &DataType,
    value: Value<AnyType>,
) -> Result<Value<AnyType>> {
    if src_type == dest_type {
        return Ok(value);
    }

    // The dest_type of `TRY_CAST` must be `Nullable`, which is guaranteed by the type checker.
    let inner_dest_type = &**dest_type.as_nullable().unwrap();

    if let Some(cast_fn) = get_simple_cast_function(true, src_type, inner_dest_type) {
        // `try_to_xxx` functions must not return errors, so we can safely call them without concerning validity.
        if let Ok(Some(new_value)) = self.run_simple_cast(
            span,
            src_type,
            dest_type,
            value.clone(),
            &cast_fn,
            None,
            &mut EvaluateOptions::default(),
        ) {
            return Ok(new_value);
        }
    }

    match (src_type, inner_dest_type) {
        (DataType::Nullable(inner_src_ty), _) => match value {
            Value::Scalar(Scalar::Null) => Ok(Value::Scalar(Scalar::Null)),
            Value::Scalar(_) => self.run_try_cast(span, inner_src_ty, dest_type, value),
            Value::Column(Column::Nullable(col)) => {
                let column = self
                    .run_try_cast(span, inner_src_ty, dest_type, Value::Column(col.column))?
                    .into_column()
                    .unwrap();
                Ok(Value::Column(NullableColumn::new_column(column, col.validity)))
            }
            other => unreachable!("source: {}", other),
        },
        (_, DataType::Nullable(inner_dest_ty)) => match value {
            Value::Scalar(scalar) => self.run_try_cast(
                span,
                src_type,
                inner_dest_ty,
                Value::Scalar(scalar),
            ),
            Value::Column(col) => {
                let column = self
                    .run_try_cast(span, src_type, inner_dest_ty, Value::Column(col))?
                    .into_column()
                    .unwrap();
                let validity = Bitmap::new_constant(true, column.len());
                Ok(Value::Column(NullableColumn::new_column(column, validity)))
            }
        },
        _ => Err(ErrorCode::BadArguments(format!(
            "unable to cast type `{src_type}` to type `{dest_type}`"
        ))
        .set_span(span)),
    }
}

This implementation ensures that nested try_cast operations are handled correctly by recursively calling run_try_cast for inner types and properly managing nullable columns. Additionally, the check_cast function in type_check.rs ensures that the destination type for TRY_CAST is always wrapped as Nullable, which is crucial for the correct functioning of nested try_cast operations [1][2][3][4].

To continue talking to Dosu, mention @dosu.