opt: remove incorrect query plans for trigram similarity filters #139265
+33
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
yuzefovich
approved these changes
Jan 17, 2025
There was a problem hiding this comment.
Nice work! I have a couple of nits to improve things a bit. I also will defer to Marcus on whether we should add an optimizer test for this or not (it doesn't seem necessary to me).
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @mgartner and @normanchenn)
-- commits line 14 at r1:
nit: I would extend the release note a bit to mention a couple things:
- that the bug was introduced in 22.2 version, and
- that 24.2+ releases are unaffected if
optimizer_use_trigram_similarity_optimizationsession variable has the default valuetrue.
pkg/sql/opt/invertedidx/trigram.go line 76 at r1 (raw file):
} // Do not plan inverted index scans when the trigram similarity threshold is 0
nit: missing period. Also I'd extend the comment a bit saying something like "because in this case all strings match the filter, yet we wouldn't perform a full scan of the inverted index".
normanchenn
force-pushed
the
norman/trigram-similarity-inverted-indexes
branch
from
2a77492 to
3d207c4
Compare
mgartner
approved these changes
Jan 24, 2025
There was a problem hiding this comment.
Reviewable status:
Previously, the optimizer would produce incorrect query plans for queries with trigram similarity filters when `pg_trgm.similarity_threshold == 0`, producing incorrect results. To address this, this patch adds a check to return early if `pg_trgm.similarity_threshold == 0` in trigram similarity queries on inverted indices. Fixes: cockroachdb#122443 Release note (bug fix): The optimizer could produce incorrect query plans for queries using trigram similarity filters (e.g. `col % 'val'`) when `pg_trgm.similarity_threshold` was set to 0. This bug was introduced in v22.2.0 and is now fixed. Note that this issue does not affect v24.2.0+ releases when the `optimizer_use_trigram_similarity_optimization` session variable (introduced in v24.2.0) is set to its default value `true`, as it would skip this behaviour.
normanchenn
force-pushed
the
norman/trigram-similarity-inverted-indexes
branch
from
3d207c4 to
496f35f
Compare
|
TFTR! bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Jan 28, 2025
139265: opt: remove incorrect query plans for trigram similarity filters r=normanchenn a=normanchenn Previously, the optimizer would produce incorrect query plans for queries with trigram similarity filters when `pg_trgm.similarity_threshold == 0`, producing incorrect results. To address this, this patch adds a check to return early if `pg_trgm.similarity_threshold == 0` in trigram similarity queries on inverted indices. Fixes: #122443 Release note (bug fix): The optimizer could produce incorrect query plans for queries using trigram similarity filters (e.g. `col % 'val'`) when `pg_trgm.similarity_threshold` was set to 0. This bug was introduced in v22.2.0 and is now fixed. Note that this issue does not affect v24.2.0+ releases when the `optimizer_use_trigram_similarity_optimization` session variable (introduced in v24.2.0) is set to its default value `true`, as it would skip this behaviour. 139914: cdctest: use legacy schema changer when sql smith is enabled r=aerfrei a=wenyihu6 This patch disables the declarative schema changer when TestChangefeedNemeses uses SQLSmith. Enabling it causes a decoder error, so it is temporarily disabled as a workaround. This ensures nemesis testing can run in CI with SQLSmith without being skipped. Informs: #137125 Release note: None Co-authored-by: Norman Chen <[email protected]> Co-authored-by: Wenyi Hu <[email protected]>
|
Build failed (retrying...): |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the optimizer would produce incorrect query plans for queries with trigram similarity filters when
pg_trgm.similarity_threshold == 0, producing incorrect results.To address this, this patch adds a check to return early if
pg_trgm.similarity_threshold == 0in trigram similarity queries on inverted indices.Fixes: #122443
Release note (bug fix): The optimizer could produce incorrect query plans for queries using trigram similarity filters (e.g.
col % 'val') whenpg_trgm.similarity_thresholdwas set to 0. This bug was introduced in v22.2.0 and is now fixed. Note that this issue does not affect v24.2.0+ releases when theoptimizer_use_trigram_similarity_optimizationsession variable (introduced in v24.2.0) is set to its default valuetrue, as it would skip this behaviour.