Skip to content

Commit 4d09c5d

Browse files
committed
tree: add InjectHints function
Add `tree.(*HintInjectionDonor).InjectHints`, which rewrites a target statement with hints copied from a donor statement. To perform the rewrite we walk both ASTs in two phases, checking that corresponding nodes match, and copying hints from donor to target. This doesn't perform any semantic checks. In a future PR we'll need to handle semantic check failures (perhaps by marking the hints as injected and treating them differently in optbuilder). (No release note because this isn't hooked up to anything yet.) Informs: #153633 Release note: None
1 parent d384eba commit 4d09c5d

File tree

5 files changed

+532
-2
lines changed

5 files changed

+532
-2
lines changed

pkg/sql/distsql_running_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ func TestDistSQLRunningInAbortedTxn(t *testing.T) {
168168
// We need to re-plan every time, since the plan is closed automatically
169169
// by PlanAndRun() below making it unusable across retries.
170170
p.stmt = makeStatement(stmt, clusterunique.ID{},
171-
tree.FmtFlags(Tree.QueryFormattingForFingerprintsMask.Get(&execCfg.Settings.SV)))
171+
tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(&execCfg.Settings.SV)))
172172
if err := p.makeOptimizerPlan(ctx); err != nil {
173173
t.Fatal(err)
174174
}

pkg/sql/sem/tree/BUILD.bazel

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ go_library(
7171
"grant.go",
7272
"import.go",
7373
"indexed_vars.go",
74+
"inject_hints.go",
7475
"insert.go",
7576
"inspect.go",
7677
"name_part.go",
@@ -216,6 +217,7 @@ go_test(
216217
"function_name_test.go",
217218
"helpers_test.go",
218219
"indexed_vars_test.go",
220+
"inject_hints_test.go",
219221
"json_test.go",
220222
"main_test.go",
221223
"name_part_test.go",
@@ -237,16 +239,22 @@ go_test(
237239
"typing_test.go",
238240
"var_expr_test.go",
239241
],
240-
data = glob(["testdata/**"]) + ["//pkg/sql/parser:sql.y"],
242+
data = glob(["testdata/**"]) + [
243+
"//c-deps:libgeos",
244+
"//pkg/sql/parser:sql.y",
245+
],
241246
embed = [":tree"],
242247
deps = [
248+
"//pkg/base",
243249
"//pkg/build/bazel",
244250
"//pkg/col/coldata",
245251
"//pkg/col/coldataext",
246252
"//pkg/internal/rsg",
253+
"//pkg/internal/sqlsmith",
247254
"//pkg/kv/kvserver/concurrency/isolation",
248255
"//pkg/security/securityassets",
249256
"//pkg/security/securitytest",
257+
"//pkg/server",
250258
"//pkg/settings/cluster",
251259
"//pkg/sql/colconv",
252260
"//pkg/sql/parser",
@@ -264,6 +272,7 @@ go_test(
264272
"//pkg/sql/types",
265273
"//pkg/testutils",
266274
"//pkg/testutils/datapathutils",
275+
"//pkg/testutils/serverutils",
267276
"//pkg/testutils/skip",
268277
"//pkg/testutils/sqlutils",
269278
"//pkg/util/duration",

pkg/sql/sem/tree/inject_hints.go

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package tree
7+
8+
import (
9+
"reflect"
10+
11+
"github.com/cockroachdb/cockroach/pkg/settings"
12+
"github.com/cockroachdb/errors"
13+
)
14+
15+
// HintInjectionDonor holds the donor statement that provides the hints to be
16+
// injected into a target statement.
17+
type HintInjectionDonor struct {
18+
// ast is the donor statement AST after parsing.
19+
ast Statement
20+
21+
// validationSQL is the donor AST formatted such that we can safely compare it
22+
// with the target statement for validation. We'll only inject hints if the
23+
// statements match.
24+
validationSQL string
25+
26+
// walk is a pre-order traversal of the AST, holding Expr and TableExpr. As we
27+
// walk the target AST we expect to find matching nodes in the same order.
28+
walk []any
29+
}
30+
31+
// validationFmtFlags returns the FmtFlags used to check that the donor
32+
// statement and the target statement match. These should be the same FmtFlags
33+
// used for statement fingerprints (including
34+
// sql.stats.statement_fingerprint.format_mask) with FmtHideHints added.
35+
func validationFmtFlags(sv *settings.Values) FmtFlags {
36+
stmtFingerprintFmtMask := FmtHideConstants | FmtFlags(QueryFormattingForFingerprintsMask.Get(sv))
37+
return stmtFingerprintFmtMask | FmtHideHints
38+
}
39+
40+
// NewHintInjectionDonor creates a HintInjectionDonor from a parsed AST.
41+
func NewHintInjectionDonor(ast Statement, sv *settings.Values) (*HintInjectionDonor, error) {
42+
donor := &HintInjectionDonor{
43+
ast: ast,
44+
validationSQL: AsStringWithFlags(ast, validationFmtFlags(sv)),
45+
}
46+
if _, err := ExtendedSimpleStmtVisit(
47+
ast,
48+
func(expr Expr) (bool, Expr, error) {
49+
donor.walk = append(donor.walk, expr)
50+
return true, expr, nil
51+
},
52+
func(expr TableExpr) (bool, TableExpr, error) {
53+
donor.walk = append(donor.walk, expr)
54+
return true, expr, nil
55+
},
56+
); err != nil {
57+
return nil, err
58+
}
59+
return donor, nil
60+
}
61+
62+
type hintInjectionVisitor struct {
63+
donor *HintInjectionDonor
64+
walkIdx int
65+
err error
66+
}
67+
68+
func (v *hintInjectionVisitor) VisitPre(expr Expr) (recurse bool, newExpr Expr) {
69+
if v.err != nil {
70+
return false, expr
71+
}
72+
73+
if v.walkIdx >= len(v.donor.walk) {
74+
v.err = errors.Newf(
75+
"hint injection donor statement missing AST node corresponding to AST node %v", expr,
76+
)
77+
return false, expr
78+
}
79+
80+
donorExpr := v.donor.walk[v.walkIdx]
81+
v.walkIdx += 1
82+
83+
// Check that the corresponding donor Expr matches this node.
84+
if reflect.TypeOf(expr) != reflect.TypeOf(donorExpr) {
85+
v.err = errors.Newf(
86+
"hint injection donor statement AST node %v did not match AST node %v", donorExpr, expr,
87+
)
88+
return false, expr
89+
}
90+
return true, expr
91+
}
92+
93+
func (v *hintInjectionVisitor) VisitTablePre(expr TableExpr) (recurse bool, newExpr TableExpr) {
94+
if v.err != nil {
95+
return false, expr
96+
}
97+
98+
if v.walkIdx >= len(v.donor.walk) {
99+
v.err = errors.Newf(
100+
"hint injection donor statement missing AST node corresponding to AST node", expr,
101+
)
102+
return false, expr
103+
}
104+
105+
donorExpr := v.donor.walk[v.walkIdx]
106+
v.walkIdx += 1
107+
108+
// Copy hints from the corresponding donor TableExpr.
109+
switch donor := donorExpr.(type) {
110+
case *AliasedTableExpr:
111+
switch orig := expr.(type) {
112+
case *AliasedTableExpr:
113+
if donor.IndexFlags != nil {
114+
// Create a new node with any pre-existing inline hints replaced with
115+
// hints from the donor.
116+
newNode := *orig
117+
newNode.IndexFlags = donor.IndexFlags
118+
return true, &newNode
119+
}
120+
return true, expr
121+
case TableExpr:
122+
if donor.IndexFlags != nil {
123+
// Wrap the node in the donor hints.
124+
newNode := &AliasedTableExpr{
125+
Expr: orig,
126+
IndexFlags: donor.IndexFlags,
127+
}
128+
return true, newNode
129+
}
130+
return false, expr
131+
}
132+
case *JoinTableExpr:
133+
switch orig := expr.(type) {
134+
case *JoinTableExpr:
135+
if donor.Hint != "" {
136+
// Create a new node with any pre-existing inline hints replaced with
137+
// hints from the donor.
138+
newNode := *orig
139+
newNode.Hint = donor.Hint
140+
return true, &newNode
141+
}
142+
return true, expr
143+
}
144+
}
145+
146+
// Check that the corresponding donor Expr matches this node.
147+
if reflect.TypeOf(expr) != reflect.TypeOf(donorExpr) {
148+
v.err = errors.Newf(
149+
"hint injection donor statement AST node %v did not match AST node %v", donorExpr, expr,
150+
)
151+
return false, expr
152+
}
153+
return true, expr
154+
}
155+
156+
func (v *hintInjectionVisitor) VisitPost(expr Expr) Expr { return expr }
157+
func (v *hintInjectionVisitor) VisitTablePost(expr TableExpr) TableExpr { return expr }
158+
159+
var _ ExtendedVisitor = &hintInjectionVisitor{}
160+
161+
// Validate checks that the target statement exactly matches the donor (except
162+
// for hints).
163+
func (hd *HintInjectionDonor) Validate(stmt Statement, sv *settings.Values) error {
164+
sql := AsStringWithFlags(stmt, validationFmtFlags(sv))
165+
if sql != hd.validationSQL {
166+
return errors.Newf(
167+
"statement does not match hint donor statement: %v vs %v", sql, hd.validationSQL,
168+
)
169+
}
170+
return nil
171+
}
172+
173+
// InjectHints rewrites the target statement with hints from the donor. Hints
174+
// from the donor replace inline hints in the target statement at corresponding
175+
// AST nodes, but other inline hints in the target statement are kept.
176+
//
177+
// True is returned if hints were copied from the donor. False is returned if no
178+
// hints were copied from the donor and no rewrite took place. If all donor
179+
// hints already existed in the target statement, they are still rewritten and
180+
// true is still returned.
181+
//
182+
// It is assumed that Validate was already called on the target statement.
183+
//
184+
// No semantic checks of the donor hints are performed. It is assumed that
185+
// semantic checks happen elsewhere.
186+
func (hd *HintInjectionDonor) InjectHints(stmt Statement) (Statement, bool, error) {
187+
// Walk the given statement, copying hints from the hints donor.
188+
v := hintInjectionVisitor{donor: hd}
189+
newStmt, changed := WalkStmt(&v, stmt)
190+
if v.err != nil {
191+
return stmt, false, v.err
192+
}
193+
if !changed {
194+
return stmt, false, nil
195+
}
196+
return newStmt, true, nil
197+
}

0 commit comments

Comments
 (0)