Skip to content

Conversation

blathers-crl[bot]
Copy link

@blathers-crl blathers-crl bot commented Aug 27, 2025

Backport 1/1 commits from #152399 on behalf of @mw5h.


Previously, when using a lookup join on an index with a virtual column,
we would require the references inside that virtual column to be typed
identically on the left and right side. This was a change from our
previous behavior, which was to require they be equivalent, but not
identical. This tightening of the requirements was done to address issue
124732, which pertains to virtual columns using functions that are
senstive to the actual type of the input (e.g. 'pg_typeof').

By tightening the reference requirement, we excluded a set of queries
from using lookup joins that had previously been able to do so safely,
regressing their performance.

In this patch, we add a cast around the left hand side reference to the
right hand side's type, so that type sensitive functions return the
correct type. This allows us to go back to the old behavior of allowing
the types of these references to be equivalent rather than identical.

Fixes: #124732
Release note (performance improvement): Lookup joins can now be used on
tables with virtual columns even if the type of the search argument is
not identical to the column type referenced in the virtual column.


Release justification: Fixes a performance regression seen by a customer.

Previously, when using a lookup join on an index with a virtual column,
we would require the references inside that virtual column to be typed
identically on the left and right side. This was a change from our
previous behavior, which was to require they be equivalent, but not
identical. This tightening of the requirements was done to address issue
124732, which pertains to virtual columns using functions that are
senstive to the actual type of the input (e.g. 'pg_typeof').

By tightening the reference requirement, we excluded a set of queries
from using lookup joins that had previously been able to do so safely,
regressing their performance.

In this patch, we add a cast around the left hand side reference to the
right hand side's type, so that type sensitive functions return the
correct type. This allows us to go back to the old behavior of allowing
the types of these references to be equivalent rather than identical.

Fixes: #147642
Informs: #124732
Release note (performance improvement): Lookup joins can now be used on
tables with virtual columns even if the type of the search argument is
not identical to the column type referenced in the virtual column.
@blathers-crl blathers-crl bot force-pushed the blathers/backport-release-24.3-152399 branch from 846f1be to 471bb52 Compare August 27, 2025 22:40
@blathers-crl blathers-crl bot requested a review from a team as a code owner August 27, 2025 22:40
@blathers-crl blathers-crl bot requested review from yuzefovich and removed request for a team August 27, 2025 22:40
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Aug 27, 2025
Copy link
Author

blathers-crl bot commented Aug 27, 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 Aug 27, 2025
Copy link
Author

blathers-crl bot commented Aug 27, 2025

❌ PR #152630 does not comply with backport policy

Confidence: high
Explanation: The pull request modifies production code related to SQL optimization and lookup joins without gating the changes behind a feature flag, thus not complying with the backport policy. The changes are not exclusively focused on non-production files, and while the PR intends to fix a performance regression, it needs to be gated behind a feature flag since there's no indication that this performance issue also constitutes a stability or security issue, nor does it result in incorrect results or suboptimal performance that would block key functionality. Although the change improves performance for specific lookup scenarios involving virtual columns, this situation does not meet backport exceptions for a critical bug as described under stability, data loss, or significant suboptimal performance that entirely breaks functionality or leads to incorrect results. The release justification states that it fixes a performance regression, but the criteria for critical bugs needing no feature gate is not met.
Recommendation: Gate the PR behind a feature flag or reconsider the backport

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

@cockroach-teamcity
Copy link
Member

This change is Reviewable

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:

@michae2 reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball, @rytaft, and @yuzefovich)

@mw5h mw5h merged commit 079127c into release-24.3 Sep 2, 2025
15 of 16 checks passed
@mw5h mw5h deleted the blathers/backport-release-24.3-152399 branch September 2, 2025 20:21
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 v24.3.20
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants