microbluey commented on PR #5125:
URL: https://github.com/apache/calcite/pull/5125#issuecomment-5094221588
Thanks for looking. Both answered, and I've pushed a commit for the first.
**"What is DEE?"** — fair challenge: the abbreviation appears nowhere else
in the codebase, so it shouldn't be in a comment. It's the standard
relational-algebra name for the relation with no attributes and one tuple (the
identity for cross join; DUM is its empty counterpart), but I've replaced it
with a plain description — "a Values with an empty row type and a single empty
tuple, which returns one row with zero columns" — in `269e562`.
**"Can this be reproduced with a quidem test?"** — I tried, and I don't
think it can, which I should have said in the description rather than leaving
you to ask.
You're right that not every builder-constructible plan surfaces from a real
program. I attempted the SQL routes to a zero-column join input:
```
-- (select) as u -> parse error, not valid syntax
select cast(a.x as varchar) from (values (1)) as a(x)
cross join (select count(*) from (values (1),(2))) as b; -- runs fine
select cast(a.x as varchar) as c from (values (1)) as a(x)
cross join (select 1 from (values (1),(2)) group by ()) as b; -- runs fine
```
Both execute correctly. The aggregate keeps a column in the plan the
validator produces, so the workaround always has a first column to fall back on
and the crash never triggers. `RelFieldTrimmer` deliberately avoids handing
anything a zero-column row type for the same historical reason as this
workaround (see the "Fennel abhors an empty row type" comment at
`RelFieldTrimmer.java:1031`), which is presumably why no SQL path reaches it.
So the reachable route is via `RelBuilder` or an equivalent frontend rather
than Calcite's own SQL: the reporter hit it through substrait-java, where an
`Aggregate(GROUP BY (), measures)` with `Rel.Emit{outputMapping=[]}` yields
exactly this shape on one side of a cross join. Since `PushProjector` guards
defensively elsewhere and the fix is two bounds checks, guarding here seemed
worth it even though Calcite's own parser can't get there — but if you'd rather
this be fixed at the frontend, or declined as unreachable, that's a reasonable
call and I'm happy to close it.
--
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]