Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

opt: query returns error when DISTINCT ON used with ASC NULLS LAST or DESC NULLS FIRST #107839

Closed
rytaft opened this issue Jul 28, 2023 · 0 comments · Fixed by #107842
Closed

opt: query returns error when DISTINCT ON used with ASC NULLS LAST or DESC NULLS FIRST #107839

rytaft opened this issue Jul 28, 2023 · 0 comments · Fixed by #107842
Assignees
@rytaft
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs

Comments

@rytaft
Copy link
Collaborator

rytaft commented Jul 28, 2023 •
edited by cockroach-jira-scripts
Loading

To reproduce, run the following on master (or any version that supports NULLS FIRST/LAST including 22.2 and 23.1):

CREATE TABLE t1 (id int, str text);
CREATE TABLE t2 (id int, num int);

SELECT
DISTINCT ON (t2.id)
t1.id,
t1.str
FROM t1 JOIN t2 ON t1.id = t2.id
ORDER BY t2.id DESC NULLS FIRST;

This returns "ERROR: no data source matches prefix: t2 in this context". It runs successfully on Postgres.

Jira issue: CRDB-30209
@rytaft rytaft added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 28, 2023
@rytaft rytaft self-assigned this Jul 28, 2023
@github-project-automation github-project-automation bot moved this to Triage in SQL Queries Jul 28, 2023
rytaft added a commit to rytaft/cockroach that referenced this issue Jul 29, 2023
@rytaft
opt: fix error with DISTINCT ON and ORDER BY ASC NULLS LAST
7346f0a 
This commit fixes an error that could occur in the optbuilder when planning a
query with DISTINCT ON and a non-standard nulls ordering.

The optimizer supports queries with a non-standard nulls ordering by projecting
a column with the expression (col IS NULL) and adding it to the ordering. Since
we require that DISTINCT ON columns must be the prefix of any ordering columns,
we must account for the new ordering column when building DISTINCT ON. A
previous bug fix for cockroachdb#90763 caused the new column to be simply ignored when
building DISTINCT ON, but this was insufficient. We need to actually include
the new column among the DISTINCT ON columns. This commit makes that change.

Fixes cockroachdb#107839

Release note (bug fix): Fixed a spurious error "no data source matches prefix"
that could occur during planning for a query with DISTINCT ON and ORDER BY ASC
NULLS LAST or ORDER BY DESC NULLS FIRST.
@rytaft rytaft mentioned this issue Jul 29, 2023
Merged
@rytaft rytaft added the O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs label Jul 29, 2023
@rytaft rytaft moved this from Triage to Active in SQL Queries Jul 29, 2023
craig bot pushed a commit that referenced this issue Aug 7, 2023
@rytaft @dt
Merge #107842 #108281
2786ad3 
107842: opt: fix error with DISTINCT ON and ORDER BY ASC NULLS LAST r=rytaft a=rytaft

This commit fixes an error that could occur in the optbuilder when planning a query with `DISTINCT ON` and a non-standard nulls ordering.

The optimizer supports queries with a non-standard nulls ordering by projecting a column with the expression `(col IS NULL)` and adding it to the ordering. Since we require that `DISTINCT ON` columns must be the prefix of any ordering columns, we must account for the new ordering column when building `DISTINCT ON`. A previous bug fix for #90763 caused the new column to be simply ignored when building `DISTINCT ON`, but this was insufficient. We need to actually include the new column among the `DISTINCT ON` columns. This commit makes that change.

Fixes #107839

Release note (bug fix): Fixed a spurious error "no data source matches prefix" that could occur during planning for a query with `DISTINCT ON` and `ORDER BY ASC NULLS LAST` or `ORDER BY DESC NULLS FIRST`.

108281: batcheval: remove stray log line r=dt a=dt

Release note: none.
Epic: none.

Fixes #108275.

Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: David Taylor <[email protected]>
@craig craig bot closed this as completed in 9dcdbcd Aug 7, 2023
@github-project-automation github-project-automation bot moved this from Active to Done in SQL Queries Aug 7, 2023
@blathers-crl blathers-crl bot mentioned this issue Aug 7, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Aug 7, 2023
@rytaft
opt: fix error with DISTINCT ON and ORDER BY ASC NULLS LAST
0116a6f 
This commit fixes an error that could occur in the optbuilder when planning a
query with DISTINCT ON and a non-standard nulls ordering.

The optimizer supports queries with a non-standard nulls ordering by projecting
a column with the expression (col IS NULL) and adding it to the ordering. Since
we require that DISTINCT ON columns must be the prefix of any ordering columns,
we must account for the new ordering column when building DISTINCT ON. A
previous bug fix for #90763 caused the new column to be simply ignored when
building DISTINCT ON, but this was insufficient. We need to actually include
the new column among the DISTINCT ON columns. This commit makes that change.

Fixes #107839

Release note (bug fix): Fixed a spurious error "no data source matches prefix"
that could occur during planning for a query with DISTINCT ON and ORDER BY ASC
NULLS LAST or ORDER BY DESC NULLS FIRST.
@blathers-crl blathers-crl bot mentioned this issue Aug 7, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Aug 7, 2023
@rytaft
opt: fix error with DISTINCT ON and ORDER BY ASC NULLS LAST
58dd728 
This commit fixes an error that could occur in the optbuilder when planning a
query with DISTINCT ON and a non-standard nulls ordering.

The optimizer supports queries with a non-standard nulls ordering by projecting
a column with the expression (col IS NULL) and adding it to the ordering. Since
we require that DISTINCT ON columns must be the prefix of any ordering columns,
we must account for the new ordering column when building DISTINCT ON. A
previous bug fix for #90763 caused the new column to be simply ignored when
building DISTINCT ON, but this was insufficient. We need to actually include
the new column among the DISTINCT ON columns. This commit makes that change.

Fixes #107839

Release note (bug fix): Fixed a spurious error "no data source matches prefix"
that could occur during planning for a query with DISTINCT ON and ORDER BY ASC
NULLS LAST or ORDER BY DESC NULLS FIRST.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rytaft rytaft

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs
Projects
SQL Queries
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

opt: fix error with DISTINCT ON and ORDER BY ASC NULLS LAST rytaft/cockroach
1 participant
@rytaft