[SQL] If an operator has multiple RetainKey successors, combine them …#5866
Open
mihaibudiu wants to merge 1 commit intomainfrom
Open
[SQL] If an operator has multiple RetainKey successors, combine them …#5866mihaibudiu wants to merge 1 commit intomainfrom
mihaibudiu wants to merge 1 commit intomainfrom
Conversation
…using MIN. This is currently only used for self-joins, but if it works well, we may enable it for other cases in the future Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
mythical-fred
approved these changes
Mar 19, 2026
Collaborator
mythical-fred
left a comment
There was a problem hiding this comment.
LGTM. Fixes the self-join GC merging gap that came up in the PR #5832 discussion. The find-then-rewrite pattern is clean, the test is well-constructed (checks both functional correctness and the GC count), and extracting createApplyN/combineMin/min as statics is a good cleanup.
Soft note: the commit subject line is ~100 chars and GitHub shows it truncated with "…". Worth squashing to ≤72 chars before merge — the full text is in the body anyway.
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.
…using MIN. This is currently only used for self-joins, but if it works well, we may enable it for other cases in the future
Fixes #5811
This is an interesting one. It's a feature we've been missing for a while: supporting multiple GC operators on the same integral.
Due to other features of the compiler, this could only happen for self-joins, and this is the case handled here. But there will be other useful applications.
This needs some infra to better test programs with GC.