sql: prohibit offset with GROUPS containing vars #38270
Conversation
|
This change is |
There was a problem hiding this comment.
Nice!
Reviewed 3 of 3 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @justinj)
pkg/sql/opt/optbuilder/testdata/window, line 1553 at r1 (raw file):
build SELECT rank() OVER (ORDER BY k GROUPS k PRECEDING) FROM kv
I'm just wondering whether the optimizer already does the check that GROUPS mode requires ORDER BY clause (I have an old PR to fix that in HP and probably in CBO as well)?
This commit adds a check during semantic analysis to verify that a window function using GROUPS mode does not include an offset expression containing variable references. Release note: None
|
TFTR! Seems we definitely don't handle that in the optimizer. Any chance you could revive your old PR? I took a look into fixing that alongside this change but it seemed like there was some stuff that needed to happen with named windows that made it non-trivial. If reviving yours isn't easy then let's open an issue for it. bors r+ |
38270: sql: prohibit offset with GROUPS containing vars r=justinj a=justinj This commit adds a check during semantic analysis to verify that a window function using GROUPS mode does not include an offset expression containing variable references. This was handled by the heuristic planner previously, but this places a check earlier so that it's also checked by the optimizer. Fixes #38090 Release note: None Co-authored-by: Justin Jaffray <justin@cockroachlabs.com>
|
I will revive that PR together with Jordan today since he's in SF. Hopefully we'll get it done :) |
Build succeeded |
This commit adds a check during semantic analysis to verify that a
window function using GROUPS mode does not include an offset expression
containing variable references. This was handled by the heuristic planner
previously, but this places a check earlier so that it's also checked by the
optimizer.
Fixes #38090
Release note: None