Skip to content

release-21.1: opt: fix SimplifyPartialIndexProjections#74475

Merged
mgartner merged 1 commit into
cockroachdb:release-21.1from
mgartner:backport21.1-74431
Jan 5, 2022
Merged

release-21.1: opt: fix SimplifyPartialIndexProjections#74475
mgartner merged 1 commit into
cockroachdb:release-21.1from
mgartner:backport21.1-74431

Conversation

@mgartner

@mgartner mgartner commented Jan 5, 2022

Copy link
Copy Markdown
Contributor

Backport 1/1 commits from #74431.

/cc @cockroachdb/release


Previously, the SimplifyPartialIndexProjections rule incorrectly
simplified partial index PUT and DEL columns that were reused for
multiple partial indexes. This caused index corruption which could
result in incorrect query results.

For example, consider:

CREATE TABLE t (
  a INT,
  b INT,
  c INT,
  INDEX a_idx (a) WHERE c IS NULL,
  INDEX b_idx (b) WHERE c IS NULL
)

UPDATE t SET a = NULL

In the UPDATE, a single column will be synthesized for the PUT and DEL
columns of both partial indexes. The UPDATE does not mutate columns in
b_idx, so the synthesized column was simplified to false. However, this
caused index corruption of a_idx because a_idx contains mutating columns
and it may require writes.

The rule has been updated so that synthesized PUT and DEL columns are
only simplified to false if they are used for a single partial index.

Fixes #74385

Release note (bug fix): A bug has been fixed which caused corruption of
partial indexes, which could cause incorrect query results. The bug was
only present when two or more partial indexes in the same table had
identical WHERE clauses. This bug has been present since version
21.1.0.


Release justification: This fixes a bug that corrupts partial indexes and
causes incorrect query results.

Previously, the SimplifyPartialIndexProjections rule incorrectly
simplified partial index PUT and DEL columns that were reused for
multiple partial indexes. This caused index corruption which could
result in incorrect query results.

For example, consider:

    CREATE TABLE t (
      a INT,
      b INT,
      c INT,
      INDEX a_idx (a) WHERE c IS NULL,
      INDEX b_idx (b) WHERE c IS NULL
    )

    UPDATE t SET a = NULL

In the UPDATE, a single column will be synthesized for the PUT and DEL
columns of both partial indexes. The UPDATE does not mutate columns in
b_idx, so the synthesized column was simplified to false. However, this
caused index corruption of a_idx because a_idx contains mutating columns
and it may require writes.

The rule has been updated so that synthesized PUT and DEL columns are
only simplified to false if they are used for a single partial index.

Fixes cockroachdb#74385

Release note (bug fix): A bug has been fixed which caused corruption of
partial indexes, which could cause incorrect query results. The bug was
only present when two or more partial indexes in the same table had
identical `WHERE` clauses. This bug has been present since version
21.1.0.
@mgartner mgartner requested review from a team, rharding6373 and rytaft January 5, 2022 21:05
@blathers-crl

blathers-crl Bot commented Jan 5, 2022

Copy link
Copy Markdown

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@cockroach-teamcity

Copy link
Copy Markdown
Member

This change is Reviewable

@rytaft rytaft left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:lgtm:

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

@mgartner mgartner merged commit 9e1e223 into cockroachdb:release-21.1 Jan 5, 2022
@mgartner mgartner deleted the backport21.1-74431 branch January 5, 2022 22:07
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.

3 participants