microbluey opened a new pull request, #5125:
URL: https://github.com/apache/calcite/pull/5125
`PushProjector.locateAllRefs` contains a workaround, originally added for
Fennel, that arbitrarily projects the first column of a `Join`/`SetOp` input
when nothing else is projected from it:
```java
if (nProject == 0 && childPreserveExprs.isEmpty()) {
projRefs.set(0);
nProject = 1;
}
```
It assumes the input *has* a first column. That fails for a zero-column
input — canonically DEE, the `Values` with an empty row type and a single empty
tuple that is the identity for cross join (it arises when an `Aggregate` with
`GROUP BY ()` has its output pruned to zero columns). The workaround then sets
a bit past the input's fields, and `createProjectRefsAndExprs` uses it to index
into an empty field list:
```
java.lang.ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0
at
org.apache.calcite.rel.rules.PushProjector.createProjectRefsAndExprs(PushProjector.java:523)
at
org.apache.calcite.rel.rules.ProjectJoinTransposeRule.onMatch(ProjectJoinTransposeRule.java:117)
```
### Fix
Guard each workaround on the input it protects having at least one field:
`nFields > 0` for the left, `nFieldsRight > 0` for the right. When there is
genuinely no first column, the workaround is skipped instead of producing an
out-of-range reference.
### Two deviations from the JIRA description, both verified
**1. The left-side case is not merely latent — it crashes too.** The ticket
notes the symmetric `nProject == 0` path "has the same latent issue" but that
only the right-side variant had been seen in practice. Putting DEE on the
*left* of the cross join throws the identical exception, so there is a
regression test for each direction.
**2. The suggested left-side guard `(nSysFields + nFields) > 0` would not be
correct.** Given the field layout documented on `nFields`:
```
| nSysFields | nFields | nFieldsRight |
```
the left input has `nFields` fields, so folding `nSysFields` into the
condition lets the workaround through when `nSysFields > 0 && nFields == 0`.
`createProjectRefsAndExprs` uses `offset = nSysFields` for the left side, so
`refIdx - offset` would then be negative — a different crash rather than a fix.
Hence `nFields > 0`.
### Note on the resulting plan
Skipping the workaround means the pushed-down projection over the
zero-column input is itself zero-column:
```
LogicalJoin(condition=[true], joinType=[inner])
LogicalProject(col1=[CAST($0):VARCHAR NOT NULL])
LogicalValues(tuples=[[{ 0 }]])
LogicalProject
LogicalValues(tuples=[[{ }]])
```
That is precisely what the Fennel workaround existed to avoid. Fennel has
long been removed, and zero-column relational expressions are legal in Calcite
today — DEE itself is one — so this seems right. Flagging it explicitly in case
anyone disagrees, since it is the behavioural part of this change.
### Out of scope, but worth recording
`projRefs.set(0)` and `projRefs.set(nFields)` look like they should be
`set(nSysFields)` and `set(nSysFields + nFields)`, since the left input's
fields start after the system fields. Demonstrating that needs a Join that
actually has system fields, and fixing it would change which column the
workaround picks, so I have deliberately kept it out of this crash fix. Happy
to file it separately if it is a real issue.
The reporter also wondered whether `ProjectCorrelateTransposeRule` has the
same latent issue. It does construct a `PushProjector`
(`ProjectCorrelateTransposeRule.java:79-80`), but the workaround is gated on
`childRel instanceof Join || childRel instanceof SetOp`, and `Correlate extends
BiRel` — it is neither. So that rule never reaches this code and is unaffected.
### Tests
Both new tests fail on current `main` with the
`ArrayIndexOutOfBoundsException` above and pass with the fix. `RelOptRulesTest`
is green (923 tests). Golden plans were produced via the standard `_actual.xml`
mechanism.
--
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]
