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.

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

Incorrect result by subquery and join #100561

Closed
DerZc opened this issue Apr 4, 2023 · 5 comments · Fixed by #103611
Closed

Incorrect result by subquery and join #100561

DerZc opened this issue Apr 4, 2023 · 5 comments · Fixed by #103611
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community S-2 Medium-high impact: many users impacted, risks of availability and difficult-to-fix data errors T-sql-queries SQL Queries Team
Projects

Comments

@DerZc
Copy link

DerZc commented Apr 4, 2023

Describe the problem

consider the following program:

CREATE DATABASE database1;
USE database1;

CREATE TABLE t0 (c0 SERIAL2);
CREATE TABLE t1 (c0 BYTES);

UPSERT INTO t1 (rowid) VALUES(1);

SELECT temp_table3.c2, temp_table3.c0 FROM (SELECT t1.c0 AS c0, t1.rowid AS c2 FROM t0 FULL OUTER JOIN t1 ON true ) AS temp_table3, t1 WHERE (temp_table3.c2) = (t1.rowid);  -- 1 | NULL

WITH temp_table3(c0, c2) AS (SELECT t1.c0 AS c0, t1.rowid AS c2 FROM t0 FULL OUTER JOIN t1 ON true ) SELECT ALL temp_table3.c2, temp_table3.c0 FROM temp_table3, t1 WHERE (temp_table3.c2) = (t1.rowid);  -- empty result

There two queries are equivalent and just put the subquery at different place. but the second query has empty results. I manually analyze it and think it should have the same result with the first query.

Also, if I remove the output column temp_table3.c0 of the second query, it will have 1 as result. This should not affect the result.

To Reproduce

this bug can reproduce with the last commit version (07c7d4b) and last release version (v22.2.7).

I run the cockroachdb in a single machine and execute this program with CLI

Expected behavior
The second query has the same results with the first one.

Environment:

  • CockroachDB version [last commit version and last release version]
  • Server OS: [ubuntu 22.04]
  • Client app [CLI]

Jira issue: CRDB-26494

@DerZc DerZc added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Apr 4, 2023
@blathers-crl
Copy link

blathers-crl bot commented Apr 4, 2023

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I was unable to automatically find someone to ping.

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner labels Apr 4, 2023
@yuzefovich yuzefovich removed the X-blathers-untriaged blathers was unable to find an owner label Apr 5, 2023
@yuzefovich yuzefovich added this to Triage in SQL Queries via automation Apr 5, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Apr 5, 2023
@yuzefovich
Copy link
Member

Thanks for the report! Indeed, both queries seem to be equivalent, and I believe they should return 1 row in the result. I also confirmed that this bug is present on master, 22.2.6, and 22.1.13, so it's not a recent regression.

@DerZc
Copy link
Author

DerZc commented Apr 5, 2023

Thank you for your confirmation. Very happy this report was helpful to you.

@mgartner mgartner self-assigned this Apr 27, 2023
@mgartner mgartner added the S-2 Medium-high impact: many users impacted, risks of availability and difficult-to-fix data errors label Apr 27, 2023
@mgartner mgartner moved this from Triage to Active in SQL Queries Apr 27, 2023
@mgartner mgartner assigned mgartner and unassigned mgartner May 8, 2023
@mgartner
Copy link
Collaborator

Here's a simplified reproduction:

CREATE TABLE a (a INT);
CREATE TABLE bc (b INT, c INT);

INSERT INTO bc (b) VALUES(1);

SELECT * FROM (
  SELECT bc.c + 1 AS y, bc.b + 1 AS x
  FROM a FULL OUTER JOIN bc ON true
) tmp, bc
WHERE tmp.x = bc.b + 1;

It returns a single row in Postgres but no rows in CRDB.

@mgartner
Copy link
Collaborator

The bug is in the RejectNullsProject rule. In some cases it can create null-rejecting filters for all of the columns that are candidates for null-rejection, not just the ones that can actually be null-rejected. The fix should be fairly simple.

I'm still trying to figure out how easy it is to hit this bug. It's been present since the RejectNullsProject rule was introduced in 20.2, which tells me that it's very rare. It's also difficult to concoct other reproductions.

mgartner added a commit to mgartner/cockroach that referenced this issue May 18, 2023
This commit fixes a bug in the `RejectNullsProject` rule that pushed
null-rejecting filters below a Project expression for _all_ projected
columns when _any_ of the projected columns had null-rejecting filters
above the projections. In other words, it could incorrectly synthesize a
null-rejecting filter for a projected column, causing incorrect results.

Fixes cockroachdb#100561

Release note (bug fix): A bug has been fixed that could cause incorrect
query results.
mgartner added a commit to mgartner/cockroach that referenced this issue May 18, 2023
This commit fixes a bug in the `RejectNullsProject` rule that pushed
null-rejecting filters below a Project expression for _all_ projected
columns when _any_ of the projected columns had null-rejecting filters
above the projections. In other words, it could incorrectly synthesize a
null-rejecting filter for a projected column, causing incorrect results.

Fixes cockroachdb#100561

Release note (bug fix): A bug has been fixed that could cause incorrect
query results. This bug was present since v20.2.
craig bot pushed a commit that referenced this issue May 23, 2023
103531: sql: enable deletes in udfs r=rharding6373 a=rharding6373

This PR enables DELETE statements in UDF bodies and adds logic test coverage of them.

Epic: CRDB-19255
Informs: #87289

Release note (sql change): Enables DELETE commands in UDF statement bodies.

103611: opt: fix bug in RejectNullsProject r=mgartner a=mgartner

#### opt: fix bug in RejectNullsProject

This commit fixes a bug in the `RejectNullsProject` rule that pushed
null-rejecting filters below a Project expression for _all_ projected
columns when _any_ of the projected columns had null-rejecting filters
above the projections. In other words, it could incorrectly synthesize a
null-rejecting filter for a projected column, causing incorrect results.

Fixes #100561

Release note (bug fix): A bug has been fixed that could cause queries
with with joins or subqueries to omit rows where column values are NULL
in very rare cases. This bug was present since v20.2.

#### opt: add fast-path to HasNullRejectingFilter

`HasNullRejectingFilter` now returns `false` immediately if the given
column set is empty, rather than unnecessarily extracting null columns
from filter constraints. This is valid because the function only returns
true if the given column set intersects with another column set, and the
empty set never intersects with any other set.

Release note: None


Co-authored-by: rharding6373 <rharding6373@users.noreply.github.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
@craig craig bot closed this as completed in 09d120d May 23, 2023
SQL Queries automation moved this from Active to Done May 23, 2023
blathers-crl bot pushed a commit that referenced this issue May 23, 2023
This commit fixes a bug in the `RejectNullsProject` rule that pushed
null-rejecting filters below a Project expression for _all_ projected
columns when _any_ of the projected columns had null-rejecting filters
above the projections. In other words, it could incorrectly synthesize a
null-rejecting filter for a projected column, causing incorrect results.

Fixes #100561

Release note (bug fix): A bug has been fixed that could cause queries
with with joins or subqueries to omit rows where column values are NULL
in very rare cases. This bug was present since v20.2.
mgartner added a commit that referenced this issue May 30, 2023
This commit fixes a bug in the `RejectNullsProject` rule that pushed
null-rejecting filters below a Project expression for _all_ projected
columns when _any_ of the projected columns had null-rejecting filters
above the projections. In other words, it could incorrectly synthesize a
null-rejecting filter for a projected column, causing incorrect results.

Fixes #100561

Release note (bug fix): A bug has been fixed that could cause queries
with with joins or subqueries to omit rows where column values are NULL
in very rare cases. This bug was present since v20.2.
mgartner added a commit that referenced this issue May 31, 2023
This commit fixes a bug in the `RejectNullsProject` rule that pushed
null-rejecting filters below a Project expression for _all_ projected
columns when _any_ of the projected columns had null-rejecting filters
above the projections. In other words, it could incorrectly synthesize a
null-rejecting filter for a projected column, causing incorrect results.

Fixes #100561

Release note (bug fix): A bug has been fixed that could cause queries
with with joins or subqueries to omit rows where column values are NULL
in very rare cases. This bug was present since v20.2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community S-2 Medium-high impact: many users impacted, risks of availability and difficult-to-fix data errors T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants