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]

Reply via email to