zelmario opened a new issue, #24388:
URL: https://github.com/apache/datafusion/issues/24388
### Describe the bug
`WITH ORDINALITY` on a table function is parsed and then discarded. No
ordinality column is added and, in most forms, nothing is reported: the query
succeeds and returns one column fewer than PostgreSQL returns for the same SQL.
An error appears only when the table alias names every column, and that
error blames the alias, which is the one part of the query that is correct:
```
Error during planning: Source table contains 1 columns but only 2 names
given as column alias
```
`TableFactor::UNNEST` already handles this correctly and returns
`not_impl_err!`. The `Table` and `Function` arms do not check the flag at all.
### To Reproduce
No postgres front end or external data needed, just a stock `SessionContext`
on 54.1.0:
```rust
use datafusion::prelude::SessionContext;
#[tokio::main]
async fn main() {
let ctx = SessionContext::new();
for sql in [
"SELECT * FROM generate_series(1,3)",
"SELECT * FROM generate_series(1,3) WITH ORDINALITY",
"SELECT * FROM generate_series(1,3) WITH ORDINALITY AS t(v)",
"SELECT * FROM generate_series(1,3) WITH ORDINALITY AS t(v, n)",
] {
println!("\n$ {sql}");
match ctx.sql(sql).await {
Ok(df) => match df.collect().await {
Ok(b) => println!("ok, {} column(s)", b.first().map(|b|
b.num_columns()).unwrap_or(0)),
Err(e) => println!("execution error: {e}"),
},
Err(e) => println!("planning error: {e}"),
}
}
}
```
Output:
```
$ SELECT * FROM generate_series(1,3)
ok, 1 column(s)
$ SELECT * FROM generate_series(1,3) WITH ORDINALITY
ok, 1 column(s)
$ SELECT * FROM generate_series(1,3) WITH ORDINALITY AS t(v)
ok, 1 column(s)
$ SELECT * FROM generate_series(1,3) WITH ORDINALITY AS t(v, n)
planning error: Error during planning: Source table contains 1 columns but
only 2 names given as column alias
```
### Expected behavior
Either the ordinality column, or a refusal. PostgreSQL 17.11 returns two
columns for all three forms:
| Query | PostgreSQL 17.11 | DataFusion 54.1.0 |
|---|---|---|
| `generate_series(1,3) WITH ORDINALITY` | 2 columns, `1\|1 2\|2 3\|3` | 1
column, no error |
| `generate_series(1,3) WITH ORDINALITY AS t(v)` | 2 columns, `1\|1 2\|2
3\|3` | 1 column, no error |
| `generate_series(1,3) WITH ORDINALITY AS t(v, n)` | 2 columns, `1\|1 2\|2
3\|3` | planning error |
PostgreSQL allows a column alias to name fewer columns than the relation
has, so `AS t(v)` is valid there and still returns the ordinality column. That
is why only the complete alias trips DataFusion's arity check: everything
shorter loses the column silently.
The silent case seems the more important half. A confusing error costs
someone an hour; a column that quietly is not there can be wrong for much
longer.
### Additional context
The flag is available on the AST node and dropped in
`datafusion-sql/src/relation/mod.rs`. `UNNEST` checks it (line 233 in 54.1.0):
```rust
TableFactor::UNNEST {
alias,
array_exprs,
with_offset: false,
with_offset_alias: None,
with_ordinality,
} => {
if with_ordinality {
return not_impl_err!("UNNEST with ordinality is not supported yet");
}
```
The other two arms let it fall into the `..`:
```rust
TableFactor::Table {
name, alias, args, .. // line 152
TableFactor::Function {
name, args, alias, .. // line 267
```
The arity error then comes from `apply_expr_alias` in
`datafusion-sql/src/planner.rs:568`, which no longer has the context to explain
why the column count is short.
A minimal fix would mirror the `UNNEST` behaviour on both arms:
```rust
if with_ordinality {
return not_impl_err!("WITH ORDINALITY is not supported yet");
}
```
That converts the silent cases into a refusal and is independent of whether
the clause is ever implemented. Implementing it properly would obviously be
better, and the two are separable. Happy to put up a PR for the `not_impl_err!`
version if that is a direction you would take.
`WITH ORDINALITY` is mentioned in passing as unsupported in #11419, in the
context of `unnest` ordering. This report is about the silent drop and the
diagnosis rather than the feature itself.
Found while comparing a PostgreSQL wire front end backed by DataFusion
against real PostgreSQL: `psql`'s `\d` sends
`pg_catalog.pg_partition_ancestors(...) WITH ORDINALITY AS a(relid, depth)`
when listing a table's triggers, so ordinary client introspection reaches it.
**Version:** DataFusion 54.1.0, sqlparser 0.62.0, Rust 1.97.1, Linux x86_64.
--
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]
