Merged
Conversation
stuhood
added
cherry-pick/0.23.x
stuhood
force-pushed
the
stuhood.window-agg-failure
branch
from
21d4d50 to
4ab3d72
Compare
stuhood
removed
the
cherry-pick/0.22.x
stuhood
force-pushed
the
stuhood.window-agg-failure
branch
from
4ab3d72 to
cb46043
Compare
stuhood
changed the title
stuhood
requested review from
mdashti,
philippemnoel and
rebasedming
as code owners
stuhood
added
the
cherry-pick/0.22.x
rebasedming
approved these changes
Jan 30, 2026
… skip window function push down.
…an, aggregatescan, and planner hook.
stuhood
force-pushed
the
stuhood.window-agg-failure
branch
from
cb46043 to
d4f1751
Compare
Contributor
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-4026-to-0.21.x
git worktree add --checkout .worktree/backport-4026-to-0.21.x backport-4026-to-0.21.x
cd .worktree/backport-4026-to-0.21.x
git reset --hard HEAD^
git cherry-pick -x 6d58de10280bd5e3fbbf1a70242ffec0d76cce42
git push --force-with-lease |
Contributor
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-4026-to-0.20.x
git worktree add --checkout .worktree/backport-4026-to-0.20.x backport-4026-to-0.20.x
cd .worktree/backport-4026-to-0.20.x
git reset --hard HEAD^
git cherry-pick -x 6d58de10280bd5e3fbbf1a70242ffec0d76cce42
git push --force-with-lease |
stuhood
added a commit
that referenced
this pull request
Feb 3, 2026
Window function replacement was: * Not a strict as TopN's order-by's constraints were. * Not disabling itself when the custom scan was disabled. Both of these cases lead to errors like: ``` ERROR: window_agg placeholder should not be executed - custom scan should have intercepted this. ``` ...either because the custom scan would run, but couldn't use TopN, or because it wouldn't run at all. Our planner hook needs to duplicate a lot of logic contained in `basescan` planning, but it needs to do so using only a `parse`/`Query`, rather than using the planning infrastructure provided to `CustomScan`s. This is error prone, and we should still try and fix it via #3455. Extracted an `orderby.rs` module in the `customscan` package to centralize the logic which is now used by the planner hook, `basescan`, and `aggregatescan`. Added new tests which failed before, and succeed now. (cherry picked from commit 6d58de1)
philippemnoel
pushed a commit
that referenced
this pull request
Feb 3, 2026
# Description Backport of #4026 to `0.21.x`. Co-authored-by: Stu Hood <[email protected]>
stuhood
added a commit
that referenced
this pull request
Feb 3, 2026
Window function replacement was: * Not a strict as TopN's order-by's constraints were. * Not disabling itself when the custom scan was disabled. Both of these cases lead to errors like: ``` ERROR: window_agg placeholder should not be executed - custom scan should have intercepted this. ``` ...either because the custom scan would run, but couldn't use TopN, or because it wouldn't run at all. Our planner hook needs to duplicate a lot of logic contained in `basescan` planning, but it needs to do so using only a `parse`/`Query`, rather than using the planning infrastructure provided to `CustomScan`s. This is error prone, and we should still try and fix it via #3455. Extracted an `orderby.rs` module in the `customscan` package to centralize the logic which is now used by the planner hook, `basescan`, and `aggregatescan`. Added new tests which failed before, and succeed now. (cherry picked from commit 6d58de1)
stuhood
added a commit
that referenced
this pull request
Feb 3, 2026
# Description Backport of #4026 to `0.20.x`. Co-authored-by: Stu Hood <[email protected]>
mdashti
pushed a commit
that referenced
this pull request
Feb 13, 2026
Backport of #4026 to `0.21.x`. Co-authored-by: Stu Hood <[email protected]>
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
3 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
Window function replacement was:
Both of these cases lead to errors like:
...either because the custom scan would run, but couldn't use TopN, or because it wouldn't run at all.
Why
Our planner hook needs to duplicate a lot of logic contained in
basescanplanning, but it needs to do so using only aparse/Query, rather than using the planning infrastructure provided toCustomScans. This is error prone, and we should still try and fix it via #3455.How
Extracted an
orderby.rsmodule in thecustomscanpackage to centralize the logic which is now used by the planner hook,basescan, andaggregatescan.Tests
Added new tests which failed before, and succeed now.