apache / datafusion

Apache DataFusion SQL Query Engine
https://datafusion.apache.org/
Apache License 2.0
6.33k stars 1.2k forks source link

Support Utf8View in Unparser `expr_to_sql` #13462

Closed phillipleblanc closed 3 days ago

phillipleblanc commented 4 days ago

Which issue does this PR close?

Closes #13461

Rationale for this change

Now that Utf8View is being returned by DataFusion, we need to ensure that when we encounter it as part of unparsing an expression, we support it properly.

What changes are included in this PR?

Correctly maps the Utf8View Arrow data type to the existing Utf8 type for unparsing.

Are these changes tested?

Yes

Are there any user-facing changes?

No API changes.

jcsherin commented 4 days ago

Maybe it will be good to add a IS NULL or IS NOT NULL test as well.

        let expr = col("a").is_not_null();

        let ast = unparser.expr_to_sql(&expr)?;
        let actual = format!("{}", ast);

        let expected = r#"a IS NOT NULL"#.to_string();

I noticed that the unparser will generate an invalid a = NULL or a <> NULL if the user writes the following expression,

let expr = col("a").eq(ScalarValue::Utf8View(None));
let ast = unparser.expr_to_sql(&expr)?;

let actual = format!("{}", ast); // a = NULL

Not a problem as long as the user correctly uses expr.is_null() or expr.is_not_null().

jcsherin commented 4 days ago

Thanks @phillipleblanc. LGTM 👍