Merge #144603

144603: tree: fix a test that wasn't seeing the error that it should have r=mw5h a=mw5h

An astute reviewer noted that the builtin_function logic test had a PREPARE with no EXECUTE and asked if that was intentional. It turns out that it was, but the PREPARE was expected to fail!

After some investigation, it was discovered that resetting use_pre_25_2_variadic_builtins was not causing CONCAT to return to the normal 25.2 behavior, which would be to error. After some poking, I discovered that the second PREPARE was not being planned at all because it was hitting the query plan cache. Flushing the plan cache in the test caused the test to correctly error.

Epic: none

Co-authored-by: Matt White <[email protected]>

Author: craig[bot]

pkg/sql/logictest/testdata/logic_test/builtin_function

@@ -223,7 +223,7 @@ EXECUTE concat_stmt2(':')
 statement ok
 RESET use_pre_25_2_variadic_builtins
 
-statement ok
+statement error pgcode 42P18 pq: concat\(\): error type checking resolved expression:: could not determine data type of placeholder \$1
 PREPARE concat_stmt3 AS SELECT concat("foo"."a", $1) FROM foo
 
 query T

pkg/sql/opt/memo/memo.go

@@ -205,6 +205,7 @@ type Memo struct {
 	planLookupJoinsWithReverseScans            bool
 	useInsertFastPath                          bool
 	internal                                   bool
+	usePre_25_2VariadicBuiltins                bool
 
 	// txnIsoLevel is the isolation level under which the plan was created. This
 	// affects the planning of some locking operations, so it must be included in
@@ -307,6 +308,7 @@ func (m *Memo) Init(ctx context.Context, evalCtx *eval.Context) {
 		planLookupJoinsWithReverseScans:            evalCtx.SessionData().OptimizerPlanLookupJoinsWithReverseScans,
 		useInsertFastPath:                          evalCtx.SessionData().InsertFastPath,
 		internal:                                   evalCtx.SessionData().Internal,
+		usePre_25_2VariadicBuiltins:                evalCtx.SessionData().UsePre_25_2VariadicBuiltins,
 		txnIsoLevel:                                evalCtx.TxnIsoLevel,
 	}
 	m.metadata.Init()
@@ -482,6 +484,7 @@ func (m *Memo) IsStale(
 		m.planLookupJoinsWithReverseScans != evalCtx.SessionData().OptimizerPlanLookupJoinsWithReverseScans ||
 		m.useInsertFastPath != evalCtx.SessionData().InsertFastPath ||
 		m.internal != evalCtx.SessionData().Internal ||
+		m.usePre_25_2VariadicBuiltins != evalCtx.SessionData().UsePre_25_2VariadicBuiltins ||
 		m.txnIsoLevel != evalCtx.TxnIsoLevel {
 		return true, nil
 	}

pkg/sql/opt/memo/memo_test.go

@@ -569,6 +569,11 @@ func TestMemoIsStale(t *testing.T) {
 	evalCtx.SessionData().Internal = false
 	notStale()
 
+	evalCtx.SessionData().UsePre_25_2VariadicBuiltins = true
+	stale()
+	evalCtx.SessionData().UsePre_25_2VariadicBuiltins = false
+	notStale()
+
 	// User no longer has access to view.
 	catalog.View(tree.NewTableNameWithSchema("t", catconstants.PublicSchemaName, "abcview")).Revoked = true
 	_, err = o.Memo().IsStale(ctx, &evalCtx, catalog)

pkg/sql/sem/tree/type_check.go

@@ -75,7 +75,7 @@ type SemaContext struct {
 	// supported by the current cluster version. It may be unset.
 	UnsupportedTypeChecker UnsupportedTypeChecker
 
-	// UsePre25_2VariadicBuiltins is set to true when we should use the pre-25.2
+	// UsePre_25_2VariadicBuiltins is set to true when we should use the pre-25.2
 	// variadic builtins behavior.
 	UsePre_25_2VariadicBuiltins bool
 }