Skip to content

Commit a8907c4

Browse files
committed
sql: do not rewrite UDF body statement slice while assigning placeholders
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.
1 parent ed0b42d commit a8907c4

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

pkg/sql/logictest/testdata/logic_test/udf_prepare

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,47 @@ PREPARE p AS SELECT $1::INT
66

77
statement error pgcode 0A000 cannot evaluate function in this context
88
EXECUTE p(f())
9+
10+
statement ok
11+
DEALLOCATE p;
12+
13+
# Ensure that stable folding does not affect plans stored in the plan cache.
14+
subtest regression_147186
15+
16+
statement ok
17+
CREATE FUNCTION f147186() RETURNS INT LANGUAGE SQL AS $$ SELECT CAST(current_setting('foo.bar') AS INT) $$;
18+
19+
statement ok
20+
CREATE TABLE t147186 (a INT, b INT DEFAULT f147186());
21+
22+
statement ok
23+
PREPARE p AS INSERT INTO t147186 (a) VALUES ($1);
24+
25+
statement ok
26+
SET foo.bar = '100';
27+
28+
statement ok
29+
EXECUTE p(1);
30+
31+
query II rowsort
32+
SELECT a, b FROM t147186;
33+
----
34+
1 100
35+
36+
statement ok
37+
SET foo.bar = '200';
38+
39+
statement ok
40+
EXECUTE p(2);
41+
42+
# The second row should reflect the custom var change.
43+
query II rowsort
44+
SELECT a, b FROM t147186;
45+
----
46+
1 100
47+
2 200
48+
49+
statement ok
50+
DEALLOCATE p;
51+
52+
subtest end

pkg/sql/opt/norm/factory.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,9 +382,20 @@ func (f *Factory) AssignPlaceholders(from *memo.Memo) (err error) {
382382
}
383383
recursiveRoutines[t.Def] = struct{}{}
384384
}
385+
// Copy the arguments, if any.
386+
var newArgs memo.ScalarListExpr
387+
if t.Args != nil {
388+
copiedArgs := f.CopyAndReplaceDefault(&t.Args, replaceFn).(*memo.ScalarListExpr)
389+
newArgs = *copiedArgs
390+
}
391+
// Make sure to copy the slice that stores the body statements, rather
392+
// than mutating the original.
393+
newDef := *t.Def
394+
newDef.Body = make([]memo.RelExpr, len(t.Def.Body))
385395
for i := range t.Def.Body {
386-
t.Def.Body[i] = f.CopyAndReplaceDefault(t.Def.Body[i], replaceFn).(memo.RelExpr)
396+
newDef.Body[i] = f.CopyAndReplaceDefault(t.Def.Body[i], replaceFn).(memo.RelExpr)
387397
}
398+
return f.ConstructUDFCall(newArgs, &memo.UDFCallPrivate{Def: &newDef})
388399
case *memo.RecursiveCTEExpr:
389400
// A recursive CTE may have the stats change on its Initial expression
390401
// after placeholder assignment, if that happens we need to

0 commit comments

Comments
 (0)