Skip to content

Commit cd60f33

Browse files
committed
opt: do not generic placeholder scans with stable functions
In v25.4.0 we introduced an optimization for some types of generic query plans which converts parameterized lookup joins into placeholder scans. Prior to this commit the spans of these placeholder scans could contain scalar expressions that were not constants nor placeholders, which execbuilder does not support. This has been fixed by being more strict in the application of this rule. Fixes #159124 Release note (bug fix): A bug has been fixed which could cause prepared statements to fail with the error message "non-const expression" when they contained filters with stable functions. This bug has been present since 25.4.0.
1 parent 86ca2bd commit cd60f33

File tree

4 files changed

+73
-18
lines changed

4 files changed

+73
-18
lines changed

pkg/sql/logictest/testdata/logic_test/generic

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,3 +690,20 @@ quality of service: regular
690690
regions: <hidden>
691691
actual row count: 0
692692
execution time: 0µs
693+
694+
# Regression test for #159124.
695+
statement ok
696+
CREATE TABLE t159124 (
697+
k INT PRIMARY KEY,
698+
s STRING,
699+
UNIQUE INDEX (s)
700+
)
701+
702+
statement ok
703+
SET plan_cache_mode = force_generic_plan
704+
705+
statement ok
706+
PREPARE p159124 AS SELECT k FROM t159124 WHERE s = current_user()
707+
708+
statement ok
709+
EXECUTE p159124

pkg/sql/opt/xform/generic_funcs.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,25 @@ func (c *CustomFuncs) GenericRulesEnabled() bool {
2323
return c.e.evalCtx.SessionData().PlanCacheMode != sessiondatapb.PlanCacheModeForceCustom
2424
}
2525

26-
// HasPlaceholdersOrStableExprs returns true if the given relational expression's subtree has
26+
// HasPlaceholders returns true if the given relational expression's subtree has
2727
// at least one placeholder.
28+
func (c *CustomFuncs) HasPlaceholders(e memo.RelExpr) bool {
29+
return e.Relational().HasPlaceholder
30+
}
31+
32+
// HasPlaceholdersOrStableExprs returns true if the given relational
33+
// expression's subtree has at least one placeholder or stable expression.
2834
func (c *CustomFuncs) HasPlaceholdersOrStableExprs(e memo.RelExpr) bool {
2935
return e.Relational().HasPlaceholder || e.Relational().VolatilitySet.HasStable()
3036
}
3137

38+
// IsConstantsAndPlaceholders returns true if all scalar expressions in the list
39+
// are constants, placeholders or tuples containing constants or placeholders.
40+
// If a tuple nested within a tuple is found, false is returned.
41+
func (c *CustomFuncs) IsConstantsAndPlaceholders(scalars memo.ScalarListExpr) bool {
42+
return scalars.IsConstantsAndPlaceholders()
43+
}
44+
3245
// GenerateParameterizedJoinValuesAndFilters returns a single-row Values
3346
// expression containing placeholders and stable expressions in the given
3447
// filters. It also returns a new set of filters where the placeholders and

pkg/sql/opt/xform/rules/generic.opt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@
8282
[]
8383
$outputCols:* &
8484
(GenericRulesEnabled) &
85-
(HasPlaceholdersOrStableExprs $values) &
85+
(HasPlaceholders $values) &
86+
(IsConstantsAndPlaceholders $row) &
8687
(Let
8788
(
8889
$span

pkg/sql/opt/xform/testdata/rules/generic

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -449,14 +449,9 @@ exec-ddl
449449
DROP INDEX partial_idx
450450
----
451451

452-
# NOTE: A placeholder scan is generated but not chosen because the cost is
453-
# higher than the lookup-join, due to the conservative row count estimate of 330
454-
# for the group with the inner project (which is the group that the placeholder
455-
# scan is added to).
456-
#
457-
# TODO(mgartner): Tighten the row count estimate of 330. By default there are
458-
# 1000 rows and 100 distinct values for t, so the row count should be ~10.
459-
opt no-stable-folds expect=(GenerateParameterizedJoin,ConvertParameterizedLookupJoinToPlaceholderScan) format=(show-stats,show-cost)
452+
# A placeholder scan's scalars values contain only constants or placeholders.
453+
# Stable functions are not supported at execbuild-time.
454+
opt no-stable-folds expect=(GenerateParameterizedJoin) expect-not=(ConvertParameterizedLookupJoinToPlaceholderScan) format=(show-stats,show-cost)
460455
SELECT k FROM t WHERE t = now()
461456
----
462457
project
@@ -536,14 +531,27 @@ project
536531
├── columns: k:1!null
537532
├── stable, has-placeholder
538533
├── key: (1)
539-
└── placeholder-scan t@t_i_t_idx
534+
└── project
540535
├── columns: k:1!null i:2!null t:5!null
541536
├── stable, has-placeholder
542537
├── key: (1)
543538
├── fd: ()-->(2,5)
544-
└── span
545-
├── $1
546-
└── now()
539+
└── inner-join (lookup t@t_i_t_idx)
540+
├── columns: k:1!null i:2!null t:5!null "$1":8!null column9:9!null
541+
├── flags: disallow merge join
542+
├── key columns: [8 9] = [2 5]
543+
├── parameterized columns: (8,9)
544+
├── stable, has-placeholder
545+
├── key: (1)
546+
├── fd: ()-->(2,5,8,9), (2)==(8), (8)==(2), (5)==(9), (9)==(5)
547+
├── values
548+
│ ├── columns: "$1":8 column9:9
549+
│ ├── cardinality: [1 - 1]
550+
│ ├── stable, has-placeholder
551+
│ ├── key: ()
552+
│ ├── fd: ()-->(8,9)
553+
│ └── ($1, now())
554+
└── filters (true)
547555

548556
opt no-stable-folds expect=GenerateParameterizedJoin
549557
SELECT * FROM t WHERE i = $1 AND t = now()
@@ -846,17 +854,33 @@ placeholder-scan t
846854
└── span
847855
└── $1
848856

849-
opt no-stable-folds expect=ConvertParameterizedLookupJoinToPlaceholderScan
857+
# A placeholder scan's scalars values contain only constants or placeholders.
858+
# Stable functions are not supported at execbuild-time.
859+
opt no-stable-folds expect-not=ConvertParameterizedLookupJoinToPlaceholderScan
850860
SELECT t.k FROM (VALUES (quote_literal(1)::INT)) AS v(k) JOIN t ON t.k = v.k
851861
----
852-
placeholder-scan t
862+
project
853863
├── columns: k:2!null
854864
├── cardinality: [0 - 1]
855865
├── stable
856866
├── key: ()
857867
├── fd: ()-->(2)
858-
└── span
859-
└── quote_literal(1)::INT8
868+
└── inner-join (lookup t)
869+
├── columns: column1:1!null k:2!null
870+
├── key columns: [1] = [2]
871+
├── lookup columns are key
872+
├── cardinality: [0 - 1]
873+
├── stable
874+
├── key: ()
875+
├── fd: ()-->(1,2), (1)==(2), (2)==(1)
876+
├── values
877+
│ ├── columns: column1:1
878+
│ ├── cardinality: [1 - 1]
879+
│ ├── stable
880+
│ ├── key: ()
881+
│ ├── fd: ()-->(1)
882+
│ └── (quote_literal(1)::INT8,)
883+
└── filters (true)
860884

861885
# The rule does not match if there are no placeholders or stable expressions in
862886
# the Values expression.

0 commit comments

Comments
 (0)