Skip to content

sql: assigning placeholders overwrites UDF body statements #147186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
DrewKimball opened this issue May 22, 2025 · 0 comments · May be fixed by #147187
Open

sql: assigning placeholders overwrites UDF body statements #147186

DrewKimball opened this issue May 22, 2025 · 0 comments · May be fixed by #147187
Assignees
@DrewKimball
Labels
branch-master Failures and bugs on the master branch. branch-release-23.2 Used to mark GA and release blockers, technical advisories, and bugs for 23.2 branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 branch-release-25.1 branch-release-25.2 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs T-sql-queries SQL Queries Team

Comments

@DrewKimball
Copy link
Collaborator

DrewKimball commented May 22, 2025 •
edited by cockroach-jira-scripts
Loading

With #141596, we now copy the body statements of a routine unconditionally when assigning placeholders. However, we missed that the slice containing the body statements also needs to be copied, or else the stable-folding that takes place during this copy step will be visible to all queries that hit the plan cache. Here's an example where this is a problem:

statement ok
CREATE FUNCTION f() RETURNS INT LANGUAGE SQL AS $$ SELECT CAST(current_setting('foo.bar') AS INT) $$;

statement ok
CREATE TABLE t (a INT, b INT DEFAULT f());

statement ok
PREPARE p AS INSERT INTO t (a) VALUES ($1);

statement ok
SET foo.bar = '100';

statement ok
EXECUTE p(1);

query II rowsort
SELECT a, b FROM t;
----
1  100

statement ok
SET foo.bar = '200';

statement ok
EXECUTE p(2);

query II rowsort
SELECT a, b FROM t;
----
1  100
2  100

In this example, the second row should have 200 for the value of b, not 100.

Jira issue: CRDB-50913
@DrewKimball DrewKimball added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs branch-master Failures and bugs on the master branch. T-sql-queries SQL Queries Team branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-23.2 Used to mark GA and release blockers, technical advisories, and bugs for 23.2 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 branch-release-25.2 branch-release-25.1 labels May 22, 2025
@github-project-automation github-project-automation bot moved this to Triage in SQL Queries May 22, 2025
@DrewKimball DrewKimball self-assigned this May 22, 2025
@DrewKimball DrewKimball moved this from Triage to Active in SQL Queries May 22, 2025
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue May 22, 2025
@DrewKimball
sql: do not rewrite UDF body statement slice while assigning placehol…
a8907c4 
…ders

Previously, we accidentally modified the original slice that contains
the body statements of a UDF while copying it during the placeholder
assignment step. As a result, constant folding that occurred in one
session could become visible in the query plan cache, causing incorrect
results. This commit fixes the bug by copying the slice as well as the
body statements.

This bug only applied to prepared statements, since we don't add plans with
stable expressions to the plan cache outside of the prepare path.

Fixes cockroachdb#147186

Release note (bug fix): Fixed a bug that could cause stable expressions
to be folded in cached query plans. The bug could cause stable expressions
like `current_setting` to return the wrong result if used in a prepared
statement. The bug was introduced in point releases v23.2.22, v24.1.14,
v24.3.9, and v25.1.2, and the v25.2 alpha.
@DrewKimball DrewKimball linked a pull request May 22, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@DrewKimball DrewKimball

Labels
branch-master Failures and bugs on the master branch. branch-release-23.2 Used to mark GA and release blockers, technical advisories, and bugs for 23.2 branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 branch-release-25.1 branch-release-25.2 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs T-sql-queries SQL Queries Team
Projects
SQL Queries
Status: Active
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: do not rewrite UDF body statement slice while assigning placeholders DrewKimball/cockroach
1 participant
@DrewKimball