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

release-19.2: opt: fix remaining filters when using partitioned constraints #44749

Merged

Conversation

@RaduBerinde
Copy link
Member

RaduBerinde commented Feb 5, 2020

Backport 1/1 commits from #44668.

/cc @cockroachdb/release


The index constraint code can use check expressions or partitioning
information to better refine the spans. However, the code related to
the handling of remaining filters is very fragile. For check
constraints, the code needs to remove any remaining filters due to
check constraints (to avoid unnecessary overhead). For the
partitioning code, this is necessary for correctness - we can't have
any "in-between" filters as part of the final remaining filters (see
big comment in GenerateConstrainedScans).

The code assumes that a remaining filter is identical to one of the
input filters (and thus only keeps the remaining filters that are the
same with an explicit filter); unfortunately, this is not necessarily
the case - the library tries to simplify the filters w.r.t the spans
to make them cheaper to evaluate.

This change cleans this up by extending the index constraints library
a little bit: we can now specify the "required" and "optional" filters
separately, with the only difference being that optional filters don't
generate remaining filters. This way we don't have to "guess" what
filters we need to keep.

Fixes #44154.

Release note (bug fix): fixed invalid query results in some corner
cases where part of a WHERE clause is incorrectly discarded.

Thanks to @mrigger for finding this bug.

The index constraint code can use check expressions or partitioning
information to better refine the spans. However, the code related to
the handling of remaining filters is very fragile. For check
constraints, the code needs to remove any remaining filters due to
check constraints (to avoid unnecessary overhead). For the
partitioning code, this is necessary for correctness - we can't have
any "in-between" filters as part of the final remaining filters (see
big comment in `GenerateConstrainedScans`).

The code assumes that a remaining filter is identical to one of the
input filters (and thus only keeps the remaining filters that are the
same with an explicit filter); unfortunately, this is not necessarily
the case - the library tries to simplify the filters w.r.t the spans
to make them cheaper to evaluate.

This change cleans this up by extending the index constraints library
a little bit: we can now specify the "required" and "optional" filters
separately, with the only difference being that optional filters don't
generate remaining filters. This way we don't have to "guess" what
filters we need to keep.

Fixes #44154.

Release note (bug fix): fixed invalid query results in some corner
cases where part of a WHERE clause is incorrectly discarded.
@RaduBerinde RaduBerinde requested a review from andy-kimball Feb 5, 2020
@RaduBerinde RaduBerinde requested a review from cockroachdb/sql-opt-prs as a code owner Feb 5, 2020
@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Feb 5, 2020

This change is Reviewable

Copy link
Contributor

andy-kimball left a comment

:lgtm:

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

@RaduBerinde RaduBerinde merged commit 28d644d into cockroachdb:release-19.2 Feb 5, 2020
3 checks passed
3 checks passed
Commit Message Lint Pull request title and commit message format are not configured
Details
GitHub CI (Cockroach) TeamCity build finished
Details
license/cla Contributor License Agreement is signed.
Details
@RaduBerinde RaduBerinde deleted the RaduBerinde:backport19.2-44668 branch Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.