fix: support aggregation with non-equality conditions in scalar subquery#24047
Closed
ck89119 wants to merge 3 commits intomatrixorigin:mainfrom
Closed
fix: support aggregation with non-equality conditions in scalar subquery#24047ck89119 wants to merge 3 commits intomatrixorigin:mainfrom
ck89119 wants to merge 3 commits intomatrixorigin:mainfrom
Conversation
…eries (matrixorigin#23942)
ck89119
changed the title
Review Summary by QodoSupport aggregation with non-equality conditions in scalar subqueries
WalkthroughsDescription• Support aggregation with non-equality conditions in scalar subqueries via correlated apply • Introduce SubqueryRunner interface to handle generic subquery execution in apply operator • Add CorrExpressionExecutor for evaluating correlated column references from outer context • Enable context passing mechanism for correlated batches through WithCorrelatedBatches Diagramflowchart LR
A["Scalar Subquery<br/>with Non-Eq Conditions"] -->|"Detect Pattern"| B["Create Apply Node"]
B -->|"Generic Subquery"| C["CorrelatedApplyRunner"]
C -->|"Evaluate"| D["CorrExpressionExecutor"]
D -->|"Access Outer Row"| E["WithCorrelatedBatches Context"]
E -->|"Return Results"| F["Apply Operator Output"]
File Changes1. pkg/sql/colexec/apply/apply.go
|
Code Review by Qodo
1. Corr vector length mismatch
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+784
to
+788
| rowCount := 1 | ||
| if len(batches) > 0 && batches[0] != nil && batches[0].RowCount() > 0 { | ||
| rowCount = batches[0].RowCount() | ||
| } | ||
| if err := expr.setConstFunc(expr.resultVector, corrBatches[relIndex].Vecs[expr.colIndex], int64(row), rowCount); err != nil { |
There was a problem hiding this comment.
1. Corr vector length mismatch 🐞 Bug ≡ Correctness
CorrExpressionExecutor.Eval returns a const vector of length 1 when the inner input batch has 0 rows, violating the invariant that expression result lengths match the input batch row count and risking downstream panics/wrong results.
Agent Prompt
### Issue description
`CorrExpressionExecutor.Eval` computes `rowCount := 1` and only updates it when the inner batch has `RowCount() > 0`. For empty inner batches, it returns a const vector with length 1, which can break downstream vectorized operators that assume vector length equals the current batch row count.
### Issue Context
Other executors generally size their outputs to `batches[0].RowCount()` (including 0). This executor should follow the same contract.
### Fix Focus Areas
- pkg/sql/colexec/evalExpression.go[784-788]
### Suggested fix
Set `rowCount` to the inner batch row count whenever an inner batch exists, even when it is 0:
- If `len(batches)>0 && batches[0]!=nil`, use `rowCount = batches[0].RowCount()`.
- Otherwise (no inner batch provided), keep `rowCount = 1` as the fallback.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+762
to
771
| if t.TableFunction != nil { | ||
| in.TableFunction = &pipeline.TableFunction{ | ||
| Attrs: t.TableFunction.Attrs, | ||
| Rets: t.TableFunction.Rets, | ||
| Args: t.TableFunction.Args, | ||
| Params: t.TableFunction.Params, | ||
| Name: t.TableFunction.FuncName, | ||
| IsSingle: t.TableFunction.IsSingle, | ||
| } | ||
| } |
There was a problem hiding this comment.
2. Runner lost in remoterun 🐞 Bug ☼ Reliability
Remote pipeline serialization/deserialization for Apply only includes TableFunction, so an Apply that relies on Runner will be reconstructed remotely with both TableFunction and Runner nil and fail with InvalidState at Prepare/Call.
Agent Prompt
### Issue description
`apply.Apply` can now operate with `Runner` instead of a `TableFunction`, but the remote pipeline instruction format only carries `TableFunction`. If an Apply-with-Runner ends up in a remote scope, it will be serialized without Runner and deserialized with both Runner and TableFunction nil, causing a runtime InvalidState error.
### Issue Context
- Serialization currently does:
- `Apply` always
- `TableFunction` only when non-nil
- **Runner is never serialized**
- Execution requires `TableFunction != nil || Runner != nil`.
### Fix Focus Areas
- pkg/sql/compile/remoterun.go[754-771]
- pkg/sql/compile/remoterun.go[1203-1218]
- pkg/sql/colexec/apply/apply.go[55-77]
### Suggested fix options (pick one)
1) **Fail fast at serialization time**: if `t.Runner != nil`, return an explicit error/NYI from `convertToPipelineInstruction` so the system never ships an un-runnable remote pipeline.
2) **Force local execution** for Apply-with-Runner scopes (compile-time), ensuring it never goes through remote serialization.
3) Add a serializable runner representation and reconstruct Runner on remote CN (likely more involved).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…copes buildReaders uses c.proc.Ctx (the compile's proc context), not the scope's own Proc.Ctx. The child compile's proc was created via NewNoContextChildProc which leaves Ctx nil. Call BuildPipelineContext on the child proc before running scopes to avoid nil-parent panic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Which issue(s) this PR fixes:
issue #23942
What this PR does / why we need it:
support aggregation with non-equality conditions in scalar subquery