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

opt: add rule to reduce IS DISTINCT FROM NULL in join filters #53180

Merged
merged 1 commit into from
Aug 22, 2020

Conversation

DrewKimball
Copy link
Collaborator

This PR introduces a normalization rule that removes an IS NOT NULL
condition on a not-null column from the ON condition of a left or full
join.

Ex:

SELECT * FROM abc FULL JOIN abc AS abc2 ON abc.a IS NOT NULL;
=>
SELECT * FROM abc FULL JOIN abc AS abc2 ON True;

Fixes #48173

Release note: None

@DrewKimball DrewKimball requested a review from a team as a code owner August 21, 2020 04:02
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@mgartner mgartner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 3 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)


pkg/sql/opt/norm/testdata/rules/join, line 2157 at r1 (raw file):

      └── a.k:1 = a2.f:9 [outer=(1,9), constraints=(/1: (/NULL - ]; /9: (/NULL - ]), fd=(1)==(9), (9)==(1)]

# Can't simplify: one equality condition has columns from same side of join.

It sounds like this is a test case we still want. This test happens to fail now because a.f=a.f is simplified to a IS DISTINCT FROM NULL, but the comment here claims that the point of the test is to ensure full-join aren't simplified when columns from the same side of the join are referenced in one of the ON filters.

I think you could test this same case by changing a.f=a.f to a.f=a.i, and it would remain a full-join even with your normalization rule.

(As an aside, this is a good reminder why comments explaining each test are extremely helpful.)

This PR introduces a normalization rule that removes an `IS NOT NULL`
condition on a not-null column from the ON condition of a left or full
join.

Ex:
```
SELECT * FROM abc FULL JOIN abc AS abc2 ON abc.a IS NOT NULL;
=>
SELECT * FROM abc FULL JOIN abc AS abc2 ON True;
```

Fixes cockroachdb#48173

Release note: None
Copy link
Collaborator Author

@DrewKimball DrewKimball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @mgartner)


pkg/sql/opt/norm/testdata/rules/join, line 2157 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

It sounds like this is a test case we still want. This test happens to fail now because a.f=a.f is simplified to a IS DISTINCT FROM NULL, but the comment here claims that the point of the test is to ensure full-join aren't simplified when columns from the same side of the join are referenced in one of the ON filters.

I think you could test this same case by changing a.f=a.f to a.f=a.i, and it would remain a full-join even with your normalization rule.

(As an aside, this is a good reminder why comments explaining each test are extremely helpful.)

Ah, nice catch. Done.

Copy link
Collaborator

@mgartner mgartner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@DrewKimball
Copy link
Collaborator Author

TFTR!

@DrewKimball
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 22, 2020

This PR was included in a batch that was canceled, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Aug 22, 2020

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 22, 2020

Build succeeded:

@craig craig bot merged commit 320eb2e into cockroachdb:master Aug 22, 2020
@DrewKimball DrewKimball deleted the distinct_from branch August 22, 2020 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opt: add rule to reduce IS DISTINCT FROM NULL in join filters
3 participants