Skip to content

Conversation

@blathers-crl
Copy link

@blathers-crl blathers-crl bot commented Dec 18, 2025

Backport 1/1 commits from #159722 on behalf of @ZhouXing19.


Fixes #159660

Previously, we don't reset the optionalMultiValFilterSuffixLen when we
find a necessary multi-val filter for a column. This would cause an incorrect
removal of an unoptional filter.

For example,

  • Index: (x, y ,z)
  • ON filters: x = 1, z IN (1, 2)
  • Optional filters: y IN (3, 4)

Since the filters are processed with the order of columns in the index,
the filters will be processed in the order of x = 1, y IN (3, 4),
z IN (1, 2).

When the processing reaches y IN (3, 4), since it is an optional filter,
optionalMultiValFilterSuffixLen = 1. But z IN (1, 2) is actually a
necessary filter, with optionalMultiValFilterSuffixLen not being reset,
it will be eventually wrongly removed.

Release note (bug fix): Fixed a bug causing a query predicate to be ignored
when the predicate is on a column following one or more ENUM columns in an
index, the predicate constrains the column to multiple values, and a lookup
join to the index is chosen for the query plan. This bug was introduced in
24.3.0 and present in all versions since.


Release justification: critical fix for the customer, fix for query plan generation that caused incorrect result.

Fixes #159660

Previously, we don't reset the `optionalMultiValFilterSuffixLen` when we
find a necessary multi-val filter for a column. This would cause an incorrect
removal of an unoptional filter.

For example,
- Index: (x, y ,z)
- ON filters: x = 1, z IN (1, 2)
- Optional filters: y IN (3, 4)

Since the filters are processed with the order of columns in the index,
the filters will be processed in the order of `x = 1`, `y IN (3, 4)`,
`z IN (1, 2)`.

When the processing reaches `y IN (3, 4)`, since it is an optional filter,
`optionalMultiValFilterSuffixLen = 1`. But `z IN (1, 2)` is actually a
necessary filter, with `optionalMultiValFilterSuffixLen` not being reset,
it will be eventually wrongly removed.

Release note (bug fix): Fixed a bug causing a query predicate to be ignored
when the predicate is on a column following one or more ENUM columns in an
index, the predicate constrains the column to multiple values, and a lookup
join to the index is chosen for the query plan. This bug was introduced in
24.3.0 and present in all versions since.
@blathers-crl blathers-crl bot requested a review from a team as a code owner December 18, 2025 00:58
@blathers-crl blathers-crl bot requested review from mw5h and removed request for a team December 18, 2025 00:58
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Dec 18, 2025
@blathers-crl blathers-crl bot requested review from mgartner and michae2 December 18, 2025 00:58
@blathers-crl
Copy link
Author

blathers-crl bot commented Dec 18, 2025

Thanks for opening a backport.

Before merging, please confirm that it falls into one of the following categories (select one):

  • Non-production code changes. Includes test-only changes, build system changes, etc.
  • Fixes for serious issues. Defined in the policy as correctness, stability, or security issues, data corruption/loss, significant performance regressions, breaking working and widely used functionality, or an inability to detect and debug production issues.
  • Other approved changes. These changes must be gated behind a disabled-by-default feature flag unless there is a strong justification not to.

Add a brief release justification to the PR description explaining your selection.

Also, confirm that the change does not break backward compatibility and complies with all aspects of the backport policy.

All backports must be reviewed by the TL and EM for the owning area.

@blathers-crl blathers-crl bot added backport Label PR's that are backports to older release branches T-sql-queries SQL Queries Team labels Dec 18, 2025
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@blathers-crl
Copy link
Author

blathers-crl bot commented Dec 18, 2025

✅ PR #159774 is compliant with backport policy

Confidence: high
Critical bug criteria met: [Bugs that can cause the DB to return incorrect results or result in suboptimal performance]
Backward compatible: true
Explanation: The pull request is compliant with the backport policy as it addresses a critical bug that can cause the database to return incorrect results due to an ignored query predicate in specific scenarios involving lookup joins and ENUM columns, which aligns with the policy's critical bug criteria of bugs causing incorrect results. The PR contains a 'Release justification: critical fix for the customer, fix for query plan generation that caused incorrect result,' which satisfies the release justification exemption, supporting its exemption from standard backport policy requirements. The changes are made in production code files under 'pkg/sql/opt', and the documentation of the bug impact and the justification provided confirms the critical nature of this bug fix.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Copy link
Collaborator

@michae2 michae2 left a comment

Choose a reason for hiding this comment

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

LGTM

@ZhouXing19 ZhouXing19 merged commit a3467f5 into release-24.3 Dec 18, 2025
16 checks passed
@ZhouXing19 ZhouXing19 deleted the blathers/backport-release-24.3-159722 branch December 18, 2025 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Label PR's that are backports to older release branches blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. T-sql-queries SQL Queries Team target-release-24.3.25

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants