sql: fix issue with variable offset in window functions#38307
sql: fix issue with variable offset in window functions#38307yuzefovich wants to merge 1 commit intocockroachdb:masterfrom
Conversation
We have fixed the issue with GROUPS mode, but ROWS and RANGE modes have the same restriction - the offset argument must not contain variables. Release note: None
|
This change is |
|
There is, however, a problem: apparently, so at least one of the logic tests fails. Justin, I need your help with it since I'm lacking the understanding of the correct terminology here :) |
justinj
left a comment
There was a problem hiding this comment.
Hm, so uncorrelated subqueries are allowed but correlated ones aren't? I guess we need to build it and check the OuterCols? I can point you in the right direction or I can do it myself, let me know which you'd prefer!
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @justinj and @yuzefovich)
pkg/sql/opt/optbuilder/scope.go, line 1148 at r1 (raw file):
var ( errOrderByIndexInWindow = pgerror.New(pgcode.FeatureNotSupported, "ORDER BY INDEX in window definition is not supported") errVarOffsetRows = pgerror.New(pgcode.Syntax, "ROWS offset cannot contain variables")
Let's just make these all one error? If variables are never allowed, seems redundant and confusing to include the mode in the error message
yuzefovich
left a comment
There was a problem hiding this comment.
I'd like to do it myself in order to get acquainted with the optimizer code a little bit, so some pointers would be appreciated :)
Reviewable status:
pkg/sql/opt/optbuilder/scope.go, line 1148 at r1 (raw file):
Previously, justinj (Justin Jaffray) wrote…
Let's just make these all one error? If variables are never allowed, seems redundant and confusing to include the mode in the error message
I followed the example of Postgres error messages, and they specify the mode in the error message. I don't have strong feeling about it though.
|
Friendly ping @justinj, please give me some guidance :) |
|
Sorry, the thing I thought would be the easy solution won't work I think, so I need to find some time to dig in and figure out what needs to happen here. |
|
Closing this PR since it doesn't work and Justin will be looking into fixing the issue. |
93483: sql: introduce hash group-join operator r=yuzefovich a=yuzefovich This PR introduces the hash group-join operator (which combines a hash join followed by a hash aggregation when the join's equality columns are the same as aggregation's grouping columns into a single operator) to the execution engines . The optimizer is currently unaware of this new operator - the changes are plumbed only from the DistSQL physical planning. Naive implementations (which simply use a hash joiner followed by a hash aggregator) are introduced to both engines with the proper disk-spilling. The usage of this new operator is gated behind an experimental session variable. See each commit for details. Addresses: #38307. Epic: None 93513: sql: add voting_replicas, non_voting_replicas columns to SHOW RANGES r=arulajmani a=ecwall Fixes #93508 Some of the multitenant admin functions accept VOTERS, NONVOTERS as input. Add voting_replicas, non_voting_replicas columns to SHOW RANGE(S) to make working with the admin functions easier. Release note (sql change): Add voting_replicas, non_voting_replicas columns to output of SHOW RANGE(S) statements. 93673: opt: allow lookup joins to preserve index ordering with DESC columns r=DrewKimball a=DrewKimball This patch fixes an oversight of #84689 that prevented lookup joins from maintaining the index ordering for each lookup if the index ordering contained descending columns. The execution logic will respect descending index columns as-is, so only the optimizer code needed to be changed. This will allow plans with lookup joins to avoid sorts in more cases. Fixes #88319 Release note (performance improvement): The optimizer can now avoid planning a sort in more cases with joins that perform lookups into an index with one or more columns sorted in descending order. This can significantly decrease the number of rows that have to be scanned in order to satisfy a `LIMIT` clause. Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com> Co-authored-by: Evan Wall <wall@cockroachlabs.com> Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
We have fixed the issue with GROUPS mode, but ROWS and RANGE modes
have the same restriction - the offset argument must not contain
variables.
Fixes: #38286.
Release note: None