[BugFix] Exclude NULL rows for negated MATCH in OR inverted-index fallback#76350
Open
Beauuu-0826 wants to merge 2 commits into
Open
[BugFix] Exclude NULL rows for negated MATCH in OR inverted-index fallback#76350Beauuu-0826 wants to merge 2 commits into
Beauuu-0826 wants to merge 2 commits into
Conversation
…lback The OR-MATCH inverted-index fallback built its bitmap from the raw positive-match posting list and flipped membership at evaluate() time for negated predicates. Because NULL rows are never present in a posting list, a negated predicate (NOT (col MATCH x)) selected them through the flip, which violates SQL three-valued logic: NOT (NULL MATCH x) is NULL, not TRUE, so the NULL rows must not be returned. Build the fallback bitmap through ColumnExprPredicate::seek_inverted_index instead, so it already is the set of rows for which the predicate is TRUE (matches intersected for a plain MATCH; matches and NULLs subtracted for a negated MATCH), consistent with the normal AND filtering path. evaluate() then selects rows by plain membership with no flip, and the rewriter treats an empty bitmap as ALWAYS_FALSE unconditionally. Drop the now-unused is_negated_expr() and make read_inverted_index() private. Signed-off-by: eason <1569163626@qq.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28067c6f55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ed-index seek seek_inverted_index cleared the row bitmap whenever the match literal was empty (read_inverted_index returned nullopt), regardless of negation. That dropped every row for NOT (col MATCH ''), even though MATCH '' matches nothing and its negation is TRUE for all non-null rows. This surfaced once the OR-MATCH fallback started reusing seek_inverted_index. Substitute an empty posting set for the nullopt case and reuse the shared with_not / intersect path, so col MATCH '' keeps no rows while NOT (col MATCH '') keeps every non-null row (only NULLs stay out). Fixes the same latent bug on the AND filtering path. Add SQL regression for both the AND and OR-MATCH fallback paths. Signed-off-by: eason <1569163626@qq.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Beauuu-0826
force-pushed
the
bugfix/gin-or-fallback-negated-null
branch
from
c006d54 to
7ceb086
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Why I'm doing:
The OR-MATCH inverted-index fallback returned NULL rows for negated MATCH
predicates nested inside an OR (e.g.
col1 MATCH 'a' OR col2 NOT MATCH 'b').Under SQL three-valued logic
NOT (NULL MATCH x)is NULL, not TRUE, so thoseNULL rows must not be returned. The canonical AND filtering path
(
ColumnExprPredicate::seek_inverted_index) already handled this correctly bysubtracting the NULL posting list; the fallback path did not.
What I'm doing:
The fallback built its bitmap from the raw positive-match posting list and
flipped membership at
evaluate()time for negated predicates. Because NULLrows are never present in a posting list, the flip selected them, producing
extra rows.
This PR builds the fallback bitmap through
ColumnExprPredicate::seek_inverted_indexinstead — the same path the normalAND filtering uses — so the bitmap already is the set of rows for which the
predicate is TRUE (matches intersected for a plain MATCH; matches and NULLs
subtracted for a negated MATCH).
evaluate()then selects rows by plainmembership with no flip, and the rewriter treats an empty bitmap as
ALWAYS_FALSEunconditionally. The now-unusedis_negated_expr()is removedand
read_inverted_index()is made private (it is only an implementationdetail of
seek_inverted_index).Tests: a C++ unit test for the no-flip
evaluate()contract, plus SQLregression cases under
test/sql/test_inverted_indexproving a negated MATCHinside an OR excludes NULL rows while a sibling OR arm can still legitimately
select them.
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: