opt: fix SimplifyPartialIndexProjections#74431
Conversation
|
This change is |
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @rharding6373)
pkg/sql/opt/norm/mutation_funcs.go, line 111 at r1 (raw file):
} // multiUsePartialIndexCols returns the set columns that are used as PUT or DEL
nit: set columns -> set of columns
pkg/sql/opt/norm/mutation_funcs.go, line 125 at r1 (raw file):
delColUsedAgain := false for j := range mp.PartialIndexPutCols {
can't you just do:
for j := 0; j < i; j++ {
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.
003b7f1 to
a5f7f26
Compare
mgartner
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/norm/mutation_funcs.go, line 111 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit: set columns -> set of columns
Done.
pkg/sql/opt/norm/mutation_funcs.go, line 125 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
can't you just do:
for j := 0; j < i; j++ {
Good point. Done.
rharding6373
left a comment
There was a problem hiding this comment.
Reviewable status:
|
TFTRs! bors r+ |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from a5f7f26 to blathers/backport-release-21.1-74431: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 21.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
|
What an amazing find! Awesome job tracking this down! |
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:
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
WHEREclauses. This bug has been present since version21.1.0.