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: simplify contradictory filters to false #85351

Merged
merged 1 commit into from Aug 1, 2022

Conversation

mgartner
Copy link
Collaborator

@mgartner mgartner commented Jul 29, 2022

The SimplifySelectFilters normalization rule now transforms filters to
false if any of the filter items are contradictions. This allows
filters to be simplified in more cases, which can trigger other rules.
This can eliminate non-leakproof expressions in filters and allow
SimplifyZeroCardinalityGroup to fire.

For example, consider the query:

SELECT a, b FROM t WHERE a > 0 AND a < 0 AND 1/b = 1

Prior to this commit, it could not be simplified to an empty values
clause because the non-leakproof division operator prevents
SimplifyZeroCardinalityGroup from applying. The normalized plan would
be:

select
 ├── columns: a:1 b:2
 ├── cardinality: [0 - 0]
 ├── immutable
 ├── scan t
 │    └── columns: a:1 b:2
 └── filters
      ├── (a:1 > 0) AND (a:1 < 0) [constraints=(contradiction)]
      └── (1 / b:2) = 1 [outer=(2), immutable]

Now, SimplifySelectFilters eliminates the division operator, making
the expression leakproof, and allowing SimplifyZeroCardinalityGroup to
apply. The normalized plan is now:

values
 ├── columns: a:1!null b:2!null
 ├── cardinality: [0 - 0]
 ├── key: ()
 └── fd: ()-->(1-2)

Release note (performance improvement): The optimizer can detect
contradictory filters in more cases, leading to more efficient query
plans.

@mgartner mgartner requested a review from a team as a code owner July 29, 2022 22:01
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@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.

Nice change! I'm surprised we hadn't been doing this already. I assume this is valid because we make no guarantees about the order in which those filters are evaluated?

:lgtm:

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

The `SimplifySelectFilters` normalization rule now transforms filters to
`false` if any of the filter items are contradictions. This allows
filters to be simplified in more cases, which can trigger other rules.
This can eliminate non-leakproof expressions in filters and allow
`SimplifyZeroCardinalityGroup` to fire.

For example, consider the query:

    SELECT a, b FROM t WHERE a > 0 AND a < 0 AND 1/b = 1

Prior to this commit, it could not be simplified to an empty values
clause because the non-leakproof division operator prevents
SimplifyZeroCardinalityGroup from applying. The normalized plan would
be:

    select
     ├── columns: a:1 b:2
     ├── cardinality: [0 - 0]
     ├── immutable
     ├── scan t
     │    └── columns: a:1 b:2
     └── filters
          ├── (a:1 > 0) AND (a:1 < 0) [constraints=(contradiction)]
          └── (1 / b:2) = 1 [outer=(2), immutable]

Now, `SimplifySelectFilters` eliminates the division operator, making
the expression leakproof, and allowing `SimplifyZeroCardinalityGroup` to
apply. The normalized plan is now:

    values
     ├── columns: a:1!null b:2!null
     ├── cardinality: [0 - 0]
     ├── key: ()
     └── fd: ()-->(1-2)

Release note (performance improvement): The optimizer can detect
contradictory filters in more cases, leading to more efficient query
plans.
Copy link
Collaborator

@rytaft rytaft left a comment

Choose a reason for hiding this comment

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

:lgtm:

@DrewKimball that's correct -- see the side-effect policy here: https://github.com/cockroachdb/cockroach/blob/662dd589831c60b2ca929eadc91e9643eb2b3cae/pkg/sql/opt/props/volatility.go

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @mgartner)

@mgartner
Copy link
Collaborator Author

mgartner commented Aug 1, 2022

TFTRs!

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 1, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 1, 2022

Build succeeded:

@craig craig bot merged commit 3be0e86 into cockroachdb:master Aug 1, 2022
@mgartner mgartner deleted the simplify-contradictions branch August 2, 2022 14:23
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.

None yet

4 participants