Skip to content

Commit 141de04

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 #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 cd8a271 commit 141de04

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
@@ -386,9 +386,20 @@ func (f *Factory) AssignPlaceholders(from *memo.Memo) (err error) {
386386
}
387387
recursiveRoutines[t.Def] = struct{}{}
388388
}
389+
// Copy the arguments, if any.
390+
var newArgs memo.ScalarListExpr
391+
if t.Args != nil {
392+
copiedArgs := f.CopyAndReplaceDefault(&t.Args, replaceFn).(*memo.ScalarListExpr)
393+
newArgs = *copiedArgs
394+
}
395+
// Make sure to copy the slice that stores the body statements, rather
396+
// than mutating the original.
397+
newDef := *t.Def
398+
newDef.Body = make([]memo.RelExpr, len(t.Def.Body))
389399
for i := range t.Def.Body {
390-
t.Def.Body[i] = f.CopyAndReplaceDefault(t.Def.Body[i], replaceFn).(memo.RelExpr)
400+
newDef.Body[i] = f.CopyAndReplaceDefault(t.Def.Body[i], replaceFn).(memo.RelExpr)
391401
}
402+
return f.ConstructUDFCall(newArgs, &memo.UDFCallPrivate{Def: &newDef})
392403
case *memo.RecursiveCTEExpr:
393404
// A recursive CTE may have the stats change on its Initial expression
394405
// after placeholder assignment, if that happens we need to

0 commit comments

Comments
 (0)