sql/*: load external statement hints from the hints cache

michae2 · michae2 · commit 3dd885370198 · 2025-10-31T15:20:44.000-07:00
When making a `sql.Statement`, call `MaybeGetStatementHints` to load any applicable external statement hints from the hint cache. We don't yet do anything with the hints once they're loaded, but we do save them in prepared statements. A few notable code paths are not yet loading hints from the hints cache: - SQL statements within UDFs and SPs - Views Informs: #153633 Release note: None
diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go
@@ -31,6 +31,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/contentionpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/execstats"
+	"github.com/cockroachdb/cockroach/pkg/sql/hints"
 	"github.com/cockroachdb/cockroach/pkg/sql/isql"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain"
@@ -383,12 +384,16 @@ func (ex *connExecutor) execStmtInOpenState(
 
 	isExtendedProtocol := prepared != nil
 	stmtFingerprintFmtMask := tree.FmtHideConstants | tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&ex.server.cfg.Settings.SV))
+	var statementHintsCache *hints.StatementHintsCache
+	if ex.executorType != executorTypeInternal {
+		statementHintsCache = ex.server.cfg.StatementHintsCache
+	}
 
 	var stmt Statement
 	if isExtendedProtocol {
-		stmt = makeStatementFromPrepared(prepared, queryID)
+		stmt = makeStatementFromPrepared(ctx, prepared, queryID, stmtFingerprintFmtMask, statementHintsCache)
 	} else {
-		stmt = makeStatement(parserStmt, queryID, stmtFingerprintFmtMask)
+		stmt = makeStatement(ctx, parserStmt, queryID, stmtFingerprintFmtMask, statementHintsCache)
 	}
 
 	if len(stmt.QueryTags) > 0 {
@@ -557,6 +562,10 @@ func (ex *connExecutor) execStmtInOpenState(
 		stmt.ExpectedTypes = ps.Columns
 		stmt.StmtNoConstants = ps.StatementNoConstants
 		stmt.StmtSummary = ps.StatementSummary
+		stmt.Hints = ps.Hints
+		stmt.HintIDs = ps.HintIDs
+		stmt.HintsGeneration = ps.HintsGeneration
+		stmt.ReloadHintsIfStale(ctx, stmtFingerprintFmtMask, statementHintsCache)
 		res.ResetStmtType(ps.AST)
 
 		if e.DiscardRows {
@@ -888,7 +897,12 @@ func (ex *connExecutor) execStmtInOpenState(
 				typeHints[i] = resolved
 			}
 		}
+		var statementHintsCache *hints.StatementHintsCache
+		if ex.executorType != executorTypeInternal {
+			statementHintsCache = ex.server.cfg.StatementHintsCache
+		}
 		prepStmt := makeStatement(
+			ctx,
 			statements.Statement[tree.Statement]{
 				// We need the SQL string just for the part that comes after
 				// "PREPARE ... AS",
@@ -901,6 +915,7 @@ func (ex *connExecutor) execStmtInOpenState(
 			},
 			ex.server.cfg.GenerateID(),
 			tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&ex.server.cfg.Settings.SV)),
+			statementHintsCache,
 		)
 		var rawTypeHints []oid.Oid
 
@@ -1252,11 +1267,15 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal(
 
 	isExtendedProtocol := portal != nil && portal.Stmt != nil
 	stmtFingerprintFmtMask := tree.FmtHideConstants | tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&ex.server.cfg.Settings.SV))
+	var statementHintsCache *hints.StatementHintsCache
+	if ex.executorType != executorTypeInternal {
+		statementHintsCache = ex.server.cfg.StatementHintsCache
+	}
 
 	if isExtendedProtocol {
-		vars.stmt = makeStatementFromPrepared(portal.Stmt, queryID)
+		vars.stmt = makeStatementFromPrepared(ctx, portal.Stmt, queryID, stmtFingerprintFmtMask, statementHintsCache)
 	} else {
-		vars.stmt = makeStatement(parserStmt, queryID, stmtFingerprintFmtMask)
+		vars.stmt = makeStatement(ctx, parserStmt, queryID, stmtFingerprintFmtMask, statementHintsCache)
 	}
 
 	var queryTimeoutTicker *time.Timer
@@ -1443,6 +1462,10 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal(
 		vars.stmt.ExpectedTypes = ps.Columns
 		vars.stmt.StmtNoConstants = ps.StatementNoConstants
 		vars.stmt.StmtSummary = ps.StatementSummary
+		vars.stmt.Hints = ps.Hints
+		vars.stmt.HintIDs = ps.HintIDs
+		vars.stmt.HintsGeneration = ps.HintsGeneration
+		vars.stmt.ReloadHintsIfStale(ctx, stmtFingerprintFmtMask, statementHintsCache)
 		res.ResetStmtType(ps.AST)
 
 		if e.DiscardRows {
@@ -1852,7 +1875,12 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal(
 				typeHints[i] = resolved
 			}
 		}
+		var statementHintsCache *hints.StatementHintsCache
+		if ex.executorType != executorTypeInternal {
+			statementHintsCache = ex.server.cfg.StatementHintsCache
+		}
 		prepStmt := makeStatement(
+			ctx,
 			statements.Statement[tree.Statement]{
 				// We need the SQL string just for the part that comes after
 				// "PREPARE ... AS",
@@ -1865,6 +1893,7 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal(
 			},
 			ex.server.cfg.GenerateID(),
 			tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&ex.server.cfg.Settings.SV)),
+			statementHintsCache,
 		)
 		var rawTypeHints []oid.Oid
 
@@ -3463,8 +3492,11 @@ func (ex *connExecutor) execStmtInNoTxnState(
 		}
 
 		p := &ex.planner
-		stmt := makeStatement(parserStmt, ex.server.cfg.GenerateID(),
-			tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&ex.server.cfg.Settings.SV)))
+		stmt := makeStatement(
+			ctx, parserStmt, ex.server.cfg.GenerateID(),
+			tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&ex.server.cfg.Settings.SV)),
+			nil, /* statementHintsCache */
+		)
 		p.stmt = stmt
 		p.semaCtx.Annotations = tree.MakeAnnotations(stmt.NumAnnotations)
 		p.extendedEvalCtx.Annotations = &p.semaCtx.Annotations
diff --git a/pkg/sql/conn_executor_prepare.go b/pkg/sql/conn_executor_prepare.go
@@ -9,6 +9,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/kv"
+	"github.com/cockroachdb/cockroach/pkg/sql/hints"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirebase"
@@ -72,8 +73,15 @@ func (ex *connExecutor) execPrepare(
 		ex.deletePreparedStmt(ctx, "")
 	}
 
-	stmt := makeStatement(parseCmd.Statement, ex.server.cfg.GenerateID(),
-		tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(ex.server.cfg.SV())))
+	var statementHintsCache *hints.StatementHintsCache
+	if ex.executorType != executorTypeInternal {
+		statementHintsCache = ex.server.cfg.StatementHintsCache
+	}
+	stmt := makeStatement(
+		ctx, parseCmd.Statement, ex.server.cfg.GenerateID(),
+		tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(ex.server.cfg.SV())),
+		statementHintsCache,
+	)
 	_, err := ex.addPreparedStmt(
 		ctx,
 		parseCmd.Name,
@@ -236,6 +244,9 @@ func (ex *connExecutor) prepare(
 		prepared.Statement.NumPlaceholders = origNumPlaceholders
 		prepared.StatementNoConstants = stmt.StmtNoConstants
 		prepared.StatementSummary = stmt.StmtSummary
+		prepared.Hints = stmt.Hints
+		prepared.HintIDs = stmt.HintIDs
+		prepared.HintsGeneration = stmt.HintsGeneration
 
 		// Point to the prepared state, which can be further populated during query
 		// preparation.
diff --git a/pkg/sql/distsql_plan_changefeed.go b/pkg/sql/distsql_plan_changefeed.go
@@ -78,11 +78,14 @@ func PlanCDCExpression(
 		opt.apply(&cfg)
 	}
 
-	p.stmt = makeStatement(statements.Statement[tree.Statement]{
-		AST: cdcExpr,
-		SQL: tree.AsString(cdcExpr),
-	}, clusterunique.ID{}, /* queryID */
+	p.stmt = makeStatement(
+		ctx,
+		statements.Statement[tree.Statement]{
+			AST: cdcExpr,
+			SQL: tree.AsString(cdcExpr),
+		}, clusterunique.ID{}, /* queryID */
 		tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&p.execCfg.Settings.SV)),
+		nil, /* statementHintsCache */
 	)
 
 	p.curPlan.init(&p.stmt, &p.instrumentation)
diff --git a/pkg/sql/distsql_running_test.go b/pkg/sql/distsql_running_test.go
@@ -167,8 +167,11 @@ func TestDistSQLRunningInAbortedTxn(t *testing.T) {
 
 		// We need to re-plan every time, since the plan is closed automatically
 		// by PlanAndRun() below making it unusable across retries.
-		p.stmt = makeStatement(stmt, clusterunique.ID{},
-			tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&execCfg.Settings.SV)))
+		p.stmt = makeStatement(
+			ctx, stmt, clusterunique.ID{},
+			tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&execCfg.Settings.SV)),
+			nil, /* statementHintsCache */
+		)
 		if err := p.makeOptimizerPlan(ctx); err != nil {
 			t.Fatal(err)
 		}
@@ -285,8 +288,11 @@ func TestDistSQLRunningParallelFKChecksAfterAbort(t *testing.T) {
 			p.ExtendedEvalContext().Tracing,
 		)
 
-		p.stmt = makeStatement(stmt, clusterunique.ID{},
-			tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&s.ClusterSettings().SV)))
+		p.stmt = makeStatement(
+			ctx, stmt, clusterunique.ID{},
+			tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&s.ClusterSettings().SV)),
+			nil, /* statementHintsCache */
+		)
 		if err := p.makeOptimizerPlan(ctx); err != nil {
 			t.Fatal(err)
 		}
diff --git a/pkg/sql/explain_tree_test.go b/pkg/sql/explain_tree_test.go
@@ -71,8 +71,11 @@ func TestPlanToTreeAndPlanToString(t *testing.T) {
 			ih.codec = execCfg.Codec
 			ih.collectBundle = true
 
-			p.stmt = makeStatement(stmt, clusterunique.ID{},
-				tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&execCfg.Settings.SV)))
+			p.stmt = makeStatement(
+				ctx, stmt, clusterunique.ID{},
+				tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&execCfg.Settings.SV)),
+				nil, /* statementHintsCache */
+			)
 			if err := p.makeOptimizerPlan(ctx); err != nil {
 				t.Fatal(err)
 			}
diff --git a/pkg/sql/hints/hint_cache.go b/pkg/sql/hints/hint_cache.go
@@ -135,6 +135,7 @@ func NewStatementHintsCache(
 			return s > int(cacheSize.Get(&st.SV))
 		},
 	})
+	hintsCache.generation.Store(1)
 	return hintsCache
 }
 
diff --git a/pkg/sql/hints/hint_cache_test.go b/pkg/sql/hints/hint_cache_test.go
@@ -416,7 +416,7 @@ func TestHintCacheGeneration(t *testing.T) {
 	}
 
 	// The initial scan should increment the generation.
-	waitForGenerationInc(0)
+	waitForGenerationInc(1)
 	generationAfterInitialScan := getGenerationAssertNoChange()
 
 	// Insert a hint - generation should increment.
diff --git a/pkg/sql/opt/optbuilder/routine.go b/pkg/sql/opt/optbuilder/routine.go
@@ -450,6 +450,8 @@ func (b *Builder) buildRoutine(
 		bodyTags = make([]string, len(stmts))
 
 		for i := range stmts {
+			// TODO(michae2): We should be checking the statement hints cache here to
+			// find any external statement hints that could apply to this statement.
 			stmtScope := b.buildStmtAtRootWithScope(stmts[i].AST, nil /* desiredTypes */, bodyScope)
 
 			// The last statement produces the output of the UDF.
diff --git a/pkg/sql/opt/optbuilder/select.go b/pkg/sql/opt/optbuilder/select.go
@@ -335,6 +335,8 @@ func (b *Builder) buildView(
 		if !ok {
 			panic(errors.AssertionFailedf("expected SELECT statement"))
 		}
+		// TODO(michae2): We should be checking the statement hints cache here to
+		// find any external statement hints that could apply to the view statement.
 
 		b.views[view] = sel
 
diff --git a/pkg/sql/plan_opt.go b/pkg/sql/plan_opt.go
@@ -7,6 +7,7 @@ package sql
 
 import (
 	"context"
+	"slices"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
@@ -139,6 +140,8 @@ func (p *planner) prepareUsingOptimizer(
 			// Check that the type hints match (the type hints affect type checking).
 			if !pm.TypeHints.Identical(p.semaCtx.Placeholders.TypeHints) {
 				opc.log(ctx, "query cache hit but type hints don't match")
+			} else if pm.HintsGeneration != stmt.Prepared.HintsGeneration && !slices.Equal(pm.HintIDs, stmt.Prepared.HintIDs) {
+				opc.log(ctx, "query cache hit but external statement hints don't match")
 			} else {
 				isStale, err := cachedData.Memo.IsStale(ctx, p.EvalContext(), opc.catalog)
 				if err != nil {
@@ -150,6 +153,9 @@ func (p *planner) prepareUsingOptimizer(
 					stmt.Prepared.StatementNoConstants = pm.StatementNoConstants
 					stmt.Prepared.Columns = pm.Columns
 					stmt.Prepared.Types = pm.Types
+					stmt.Hints = pm.Hints
+					stmt.HintIDs = pm.HintIDs
+					stmt.HintsGeneration = pm.HintsGeneration
 					if cachedData.Memo.IsOptimized() {
 						// A cache, fully optimized memo is an "ideal generic
 						// memo".
diff --git a/pkg/sql/plan_opt_test.go b/pkg/sql/plan_opt_test.go
@@ -652,7 +652,9 @@ func TestPlanGistControl(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	p.stmt = makeStatement(stmt, clusterunique.ID{}, fmtFingerprintMask)
+	p.stmt = makeStatement(
+		ctx, stmt, clusterunique.ID{}, fmtFingerprintMask, nil, /* statementHintsCache */
+	)
 	if err := p.makeOptimizerPlan(ctx); err != nil {
 		t.Fatal(err)
 	}
@@ -678,7 +680,9 @@ func TestPlanGistControl(t *testing.T) {
 	p = internalPlanner.(*planner)
 	p.SessionData().DisablePlanGists = true
 
-	p.stmt = makeStatement(stmt, clusterunique.ID{}, fmtFingerprintMask)
+	p.stmt = makeStatement(
+		ctx, stmt, clusterunique.ID{}, fmtFingerprintMask, nil, /* statementHintsCache */
+	)
 	if err := p.makeOptimizerPlan(ctx); err != nil {
 		t.Fatal(err)
 	}
diff --git a/pkg/sql/plpgsql/parser/lexer.go b/pkg/sql/plpgsql/parser/lexer.go
@@ -119,6 +119,8 @@ func (l *lexer) parseOne(sql string) (tree.Statement, error) {
 		return nil, err
 	}
 	l.numAnnotations = sqlStmt.NumAnnotations
+	// TODO(michae2): We should be checking the statement hints cache here to find
+	// any external statement hints that could apply to this statement.
 	return sqlStmt.AST, nil
 }
 
diff --git a/pkg/sql/prep/BUILD.bazel b/pkg/sql/prep/BUILD.bazel
@@ -11,6 +11,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/sql/catalog/colinfo",
+        "//pkg/sql/hintpb",
         "//pkg/sql/opt/memo",
         "//pkg/sql/parser/statements",
         "//pkg/sql/sem/tree",
diff --git a/pkg/sql/prep/metadata.go b/pkg/sql/prep/metadata.go
@@ -9,14 +9,16 @@ import (
 	"unsafe"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
+	"github.com/cockroachdb/cockroach/pkg/sql/hintpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/parser/statements"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/lib/pq/oid"
 )
 
-// Metadata encapsulates information about a statement that is gathered
-// during Prepare and is later used during Describe or Execute.
+// Metadata encapsulates information about a statement that is gathered during
+// Prepare and is later used during Describe or Execute. It should contain
+// everything necessary to create a sql.Statement at execution time.
 type Metadata struct {
 	// Note that AST may be nil if the prepared statement is empty.
 	statements.Statement[tree.Statement]
@@ -38,6 +40,19 @@ type Metadata struct {
 	// InferredTypes represents the inferred types for placeholder, using protocol
 	// identifiers. Used for reporting on Describe.
 	InferredTypes []oid.Oid
+
+	// Hints are any external statement hints from the system.statement_hints
+	// table that could apply to this statement, based on the statement
+	// fingerprint.
+	Hints []hintpb.StatementHintUnion
+
+	// HintIDs are the IDs of any external statement hints, which are used for
+	// invalidation of cached plans.
+	HintIDs []int64
+
+	// HintsGeneration is the generation of the hints cache at the time the
+	// hints were retrieved, used for invalidation of cached plans.
+	HintsGeneration int64
 }
 
 // MemoryEstimate returns an estimation (in bytes) of how much memory is used by
@@ -57,5 +72,10 @@ func (pm *Metadata) MemoryEstimate() int64 {
 	res += int64(len(pm.Columns)) * int64(unsafe.Sizeof(colinfo.ResultColumn{}))
 	res += int64(len(pm.InferredTypes)) * int64(unsafe.Sizeof(oid.Oid(0)))
 
+	for i := range pm.Hints {
+		res += int64(pm.Hints[i].Size())
+	}
+	res += int64(len(pm.HintIDs)) * int64(unsafe.Sizeof(int64(0)))
+
 	return res
 }
diff --git a/pkg/sql/querycache/query_cache_test.go b/pkg/sql/querycache/query_cache_test.go
@@ -44,7 +44,7 @@ func data(sql string, mem *memo.Memo, memEstimate int64) *CachedData {
 	cd := &CachedData{SQL: sql, Memo: mem, Metadata: &prep.Metadata{}}
 	n := memEstimate - cd.memoryEstimate()
 	if n < 0 {
-		panic(errors.AssertionFailedf("size %d too small", memEstimate))
+		panic(errors.AssertionFailedf("size %d too small %d", memEstimate, cd.memoryEstimate()))
 	}
 	// Add characters to AnonymizedStr which should increase the estimate.
 	s := make([]byte, n)
@@ -202,7 +202,7 @@ func TestSynchronization(t *testing.T) {
 					c.Purge(sql)
 				case r <= 35:
 					// 25% of the time, add an entry.
-					c.Add(&s, data(sql, &memo.Memo{}, int64(256+rng.Intn(10*avgCachedSize))))
+					c.Add(&s, data(sql, &memo.Memo{}, int64(288+rng.Intn(10*avgCachedSize))))
 				default:
 					// The rest of the time, find an entry.
 					_, _ = c.Find(&s, sql)
diff --git a/pkg/sql/schema_changer.go b/pkg/sql/schema_changer.go
diff --git a/pkg/sql/session_state.go b/pkg/sql/session_state.go
diff --git a/pkg/sql/sql_cursor.go b/pkg/sql/sql_cursor.go
diff --git a/pkg/sql/statement.go b/pkg/sql/statement.go
diff --git a/pkg/sql/statement_test.go b/pkg/sql/statement_test.go
diff --git a/pkg/sql/testutils.go b/pkg/sql/testutils.go

Original file line number	Diff line number	Diff line change
`@@ -135,6 +135,7 @@ func NewStatementHintsCache(`
`135`	`135`	`return s > int(cacheSize.Get(&st.SV))`
`136`	`136`	`},`
`137`	`137`	`})`
	`138`	`+ hintsCache.generation.Store(1)`
`138`	`139`	`return hintsCache`
`139`	`140`	`}`
`140`	`141`
Original file line number	Diff line number	Diff line change
`@@ -416,7 +416,7 @@ func TestHintCacheGeneration(t *testing.T) {`
`416`	`416`	`}`
`417`	`417`
`418`	`418`	`// The initial scan should increment the generation.`
`419`		`- waitForGenerationInc(0)`
	`419`	`+ waitForGenerationInc(1)`
`420`	`420`	`generationAfterInitialScan := getGenerationAssertNoChange()`
`421`	`421`
`422`	`422`	`// Insert a hint - generation should increment.`
Original file line number	Diff line number	Diff line change
`@@ -335,6 +335,8 @@ func (b *Builder) buildView(`
`335`	`335`	`if !ok {`
`336`	`336`	`panic(errors.AssertionFailedf("expected SELECT statement"))`
`337`	`337`	`}`
	`338`	`+ // TODO(michae2): We should be checking the statement hints cache here to`
	`339`	`+ // find any external statement hints that could apply to the view statement.`
`338`	`340`
`339`	`341`	`b.views[view] = sel`
`340`	`342`