Skip to content

Conversation

@blathers-crl
Copy link

@blathers-crl blathers-crl bot commented Nov 25, 2025

Backport 1/1 commits from #156966 on behalf of @DrewKimball.


PL/pgSQL routines use barrier expressions to prevent optimizations that would remove side effecting expressions from the query plan. Previously, we would add a barrier below the outer join used to ensure at least one row returned by a SQL statement with an INTO clause. However, this wasn't enough, since after some inlining and projection push-down into the join, the join could be eliminated. This could result in a side-effecting SQL statement being dropped if its target variable(s) weren't referenced.

This commit fixes the bug by moving the barrier above the outer-join. This prevents optimizations (like join elimination) that rely on proving that the columns from the null-extended side of the join are not needed.

Fixes #147269

Release note (bug fix): Fixed a bug existing since SQL statements with INTO clause were introduced for PL/pgSQL routines in v23.2. The bug could cause a SQL statement with side effects (e.g. INSERT) to be dropped if none of the target variables from the INTO clause were referenced.


Release justification:

PL/pgSQL routines use barrier expressions to prevent optimizations
that would remove side effecting expressions from the query plan.
Previously, we would add a barrier below the outer join used to ensure
at least one row returned by a SQL statement with an INTO clause.
However, this wasn't enough, since after some inlining and projection
push-down into the join, the join could be eliminated. This could
result in a side-effecting SQL statement being dropped if its target
variable(s) weren't referenced.

This commit fixes the bug by moving the barrier above the outer-join.
This prevents optimizations (like join elimination) that rely on
proving that the columns from the null-extended side of the join are
not needed.

Fixes #147269

Release note (bug fix): Fixed a bug existing since SQL statements with
INTO clause were introduced for PL/pgSQL routines in v23.2. The bug could
cause a SQL statement with side effects (e.g. INSERT) to be dropped if
none of the target variables from the INTO clause were referenced.
@blathers-crl blathers-crl bot requested a review from a team as a code owner November 25, 2025 19:49
@blathers-crl blathers-crl bot requested review from yuzefovich and removed request for a team November 25, 2025 19:49
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Nov 25, 2025
@blathers-crl blathers-crl bot requested review from mgartner, michae2 and mw5h November 25, 2025 19:49
@blathers-crl
Copy link
Author

blathers-crl bot commented Nov 25, 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 Nov 25, 2025
@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 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @mw5h, and @yuzefovich)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants