andygrove opened a new issue, #26316:
URL: https://github.com/apache/datafusion/issues/26316
### Describe the bug
The SQL unparser mishandles NULL literals of nested types:
- A NULL list literal unparses as `[]`, so the generated SQL returns an
empty array where the plan returns NULL, with no error.
- A NULL struct, map, or union literal fails with
`NotImplemented("Unsupported scalar: ...")`.
Optimized plans contain these literals whenever type coercion casts a `NULL`
to a nested type and constant folding turns the cast into a literal, e.g. `CASE
WHEN ... THEN NULL ELSE list_col END` or `... UNION ALL SELECT NULL`.
The cause is in `Unparser::scalar_to_sql`. The list arms render the scalar's
child values without checking whether the list itself is null
([unparser/expr.rs#L1450-L1454](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/sql/src/unparser/expr.rs#L1450-L1454)),
and the `Struct`/`Map`/`Union` arms return `not_impl_err!` even for nulls
([unparser/expr.rs#L1579-L1581](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/sql/src/unparser/expr.rs#L1579-L1581)).
Every other typed null, such as `Int32(None)` or `DurationSecond(None)`,
unparses as `NULL`.
### To Reproduce
```rust
use datafusion::error::Result;
use datafusion::prelude::*;
use datafusion::sql::unparser::plan_to_sql;
#[tokio::main]
async fn main() -> Result<()> {
let ctx = SessionContext::new();
ctx.sql("CREATE TABLE t AS SELECT 1 AS a, [1, 2] AS l, named_struct('x',
1) AS s")
.await?;
for sql in [
"SELECT CASE WHEN a > 0 THEN NULL ELSE l END AS c FROM t",
"SELECT CASE WHEN a > 0 THEN NULL ELSE s END AS c FROM t",
] {
let plan = ctx.sql(sql).await?.into_optimized_plan()?;
println!("{:?}", plan_to_sql(&plan).map(|s| s.to_string()));
}
Ok(())
}
```
Output:
```
Ok("SELECT CASE WHEN (t.a > 0) THEN [] ELSE t.l END AS c FROM t")
Err(NotImplemented("Unsupported scalar: Struct({x:})"))
```
Running the first unparsed query returns `[]` where the original returns
NULL. `SELECT l FROM t UNION ALL SELECT NULL FROM t` similarly unparses to `...
UNION ALL SELECT [] AS l FROM t`. Map columns fail the same way as struct
columns.
### Expected behavior
NULL nested-type literals unparse as `NULL`, like other typed nulls, e.g.
`CASE WHEN (t.a > 0) THEN NULL ELSE t.l END`.
A guard arm at the top of `scalar_to_sql` is enough:
```rust
ScalarValue::List(_)
| ScalarValue::LargeList(_)
| ScalarValue::FixedSizeList(_)
| ScalarValue::ListView(_)
| ScalarValue::LargeListView(_)
| ScalarValue::Struct(_)
| ScalarValue::Map(_)
| ScalarValue::Union(..)
if v.is_null() =>
{
Ok(ast::Expr::value(ast::Value::Null))
}
```
With this arm, the queries above, the `UNION ALL` case, and the map case all
unparse to SQL that returns the same results as the original query, and the
existing `datafusion-sql` tests still pass.
### Additional context
- Non-null struct and map literals, such as a constant-folded
`named_struct('x', 1)`, also fail with `Unsupported scalar`. Rendering those
needs dialect-specific syntax, so they are out of scope here.
- Once #26311 merges, `CASE WHEN a > 0 THEN s ELSE NULL END` fails the same
way. Until then the unparser drops the failing ELSE, which happens to give the
same result for `ELSE NULL`.
- Line references are pinned to commit 0d79252f61bd.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
