Skip to content

Commit 6e0b922

Browse files
committed
sql: add statement hints to statement diagnostics bundles
Fixes cockroachdb#161829 Release note (sql change): We now include `information_schema.crdb_rewrite_inline_hints` statements in the `schema.sql` file of a statement diagnostics bundle for re-creating all the statement hints bound to the statement. The hints recreation statement have been sorted in ascending order of the original hints' creation time.
1 parent 4e667e5 commit 6e0b922

File tree

6 files changed

+220
-0
lines changed

6 files changed

+220
-0
lines changed

pkg/sql/explain_bundle.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ import (
2323
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
2424
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
2525
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
26+
"github.com/cockroachdb/cockroach/pkg/sql/hintpb"
2627
"github.com/cockroachdb/cockroach/pkg/sql/lexbase"
2728
"github.com/cockroachdb/cockroach/pkg/sql/opt"
2829
"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
2930
"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain"
3031
"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
32+
"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
3133
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
3234
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
3335
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -556,6 +558,74 @@ var stmtBundleIncludeAllFKReferences = settings.RegisterBoolSetting(
556558
false,
557559
)
558560

561+
// getStmtHintRecreateStmts returns the SQL statements that can be used to
562+
// recreate all the statement hints bound to the target statement. The returned
563+
// statements are sorted in ascending order by the original hints' creation time.
564+
func (c *stmtEnvCollector) getStmtHintRecreateStmts(
565+
ctx context.Context, stmt string, sv *settings.Values,
566+
) (recreateStmts []string, err error) {
567+
fingerprint, err := parserutils.FingerprintStatement(stmt,
568+
tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(
569+
sv,
570+
)))
571+
if err != nil {
572+
return nil, err
573+
}
574+
575+
query := `SELECT hint FROM system.statement_hints WHERE fingerprint = $1 ORDER BY created_at`
576+
rows, err := c.ie.QueryBufferedEx(
577+
ctx,
578+
"stmtEnvCollector",
579+
nil, /* txn */
580+
c.ieo,
581+
query,
582+
fingerprint,
583+
)
584+
if err != nil {
585+
return nil, err
586+
}
587+
588+
recreateStmts = make([]string, 0, len(rows))
589+
for _, row := range rows {
590+
if len(row) == 0 {
591+
return nil, errors.AssertionFailedf("expect not nil row")
592+
}
593+
if row[0] == tree.DNull {
594+
continue
595+
}
596+
hintBytes, ok := row[0].(*tree.DBytes)
597+
if !ok {
598+
return nil, errors.AssertionFailedf("expected hint to be bytes, got %T", row[0])
599+
}
600+
// Deserialize from the protobuf.
601+
hint, err := hintpb.FromBytes([]byte(*hintBytes))
602+
if err != nil {
603+
return nil, errors.Wrap(err, "deserializing statement hint")
604+
}
605+
recreateSQL, ok := hint.RecreateStmt(stmt)
606+
if !ok {
607+
continue
608+
}
609+
recreateStmts = append(recreateStmts, recreateSQL)
610+
}
611+
return recreateStmts, nil
612+
}
613+
614+
// PrintStmtHints writes the SQL statements needed to recreate the statement
615+
// hints for the given statement to w.
616+
func (c *stmtEnvCollector) PrintStmtHints(
617+
ctx context.Context, stmt string, sv *settings.Values, w io.Writer,
618+
) error {
619+
recreateStmts, err := c.getStmtHintRecreateStmts(ctx, stmt, sv)
620+
if err != nil {
621+
return err
622+
}
623+
for _, s := range recreateStmts {
624+
fmt.Fprintf(w, "%s\n", s)
625+
}
626+
return nil
627+
}
628+
559629
// stmtBundleStatsFileRE is a regex that matches all "complex" characters that
560630
// might not be safe when used in file names on some systems.
561631
var stmtBundleStatsFileRE = regexp.MustCompile(`[^a-zA-Z0-9_.\-]`)
@@ -1007,6 +1077,13 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) {
10071077
b.printError(fmt.Sprintf("-- error getting schema for view %s: %v", views[i].FQString(), err), &buf)
10081078
}
10091079
}
1080+
1081+
if !b.flags.RedactValues {
1082+
if err := c.PrintStmtHints(ctx, b.stmt, b.sv, &buf); err != nil {
1083+
b.printError(fmt.Sprintf("-- error getting statement hints: %v", err), &buf)
1084+
}
1085+
}
1086+
10101087
if buf.Len() == 0 {
10111088
buf.WriteString("-- there were no objects used in this query\n")
10121089
}

pkg/sql/explain_bundle_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,7 @@ CREATE TABLE users(id UUID DEFAULT gen_random_uuid() PRIMARY KEY, promo_id INT R
11451145
r.Exec(t, "CREATE POLICY policy1 ON rls1 USING (u = 'hal')")
11461146
r.Exec(t, "CREATE USER rls_user")
11471147
r.Exec(t, "GRANT SYSTEM VIEWCLUSTERSETTING TO rls_user")
1148+
r.Exec(t, "GRANT SYSTEM VIEWSYSTEMTABLE TO rls_user")
11481149
r.Exec(t, "ALTER TABLE rls1 OWNER TO rls_user")
11491150
r.Exec(t, "SET ROLE rls_user")
11501151
defer r.Exec(t, "SET ROLE root")
@@ -1178,6 +1179,61 @@ CREATE TABLE users(id UUID DEFAULT gen_random_uuid() PRIMARY KEY, promo_id INT R
11781179
)
11791180
})
11801181

1182+
t.Run("statement hints", func(t *testing.T) {
1183+
r.Exec(t, "CREATE TABLE table161829(x INT PRIMARY KEY, y INT, z STRING)")
1184+
r.Exec(t, "CREATE INDEX xy161829 ON table161829 (x, y)")
1185+
r.Exec(t, "CREATE INDEX y161829 ON table161829 (y)")
1186+
r.Exec(t, "CREATE INDEX xz161829 ON table161829 (x, z)")
1187+
r.Exec(t, `SELECT information_schema.crdb_rewrite_inline_hints('SELECT * FROM table161829 WHERE y = 10', 'SELECT * FROM table161829@primary WHERE y = 10')`)
1188+
r.Exec(t, `SELECT information_schema.crdb_rewrite_inline_hints('SELECT * FROM table161829 WHERE y = 10', 'SELECT * FROM table161829@xy161829 WHERE y = 10')`)
1189+
r.Exec(t, `SELECT information_schema.crdb_rewrite_inline_hints('SELECT * FROM table161829 WHERE y = 10', 'SELECT * FROM table161829@y161829 WHERE y = 10')`)
1190+
1191+
rows := r.QueryStr(t, "EXPLAIN ANALYZE (DEBUG) SELECT * FROM table161829 WHERE y = 10")
1192+
checkBundle(
1193+
t, fmt.Sprint(rows), "table161829", func(name, contents string) error {
1194+
if name == "schema.sql" {
1195+
reg := regexp.MustCompile(`crdb_rewrite_inline_hints\(.*\@primary.*\n.*crdb_rewrite_inline_hints.*\@xy161829.*\n.*crdb_rewrite_inline_hints.*\@y161829`)
1196+
if reg.FindString(contents) == "" {
1197+
return errors.Errorf("could not find full crdb_rewrite_inline_hints in schema.sql")
1198+
}
1199+
}
1200+
return nil
1201+
}, false, /* expectErrors */
1202+
base, plans, "distsql.html vec.txt vec-v.txt stats-defaultdb.public.table161829.sql",
1203+
)
1204+
1205+
// Bundle for statements that share the same fingerprint should all see the same hints.
1206+
rows = r.QueryStr(t, "EXPLAIN ANALYZE (DEBUG) SELECT * FROM table161829 WHERE y = 100")
1207+
checkBundle(
1208+
t, fmt.Sprint(rows), "table161829", func(name, contents string) error {
1209+
if name == "schema.sql" {
1210+
reg := regexp.MustCompile(`crdb_rewrite_inline_hints\(.*\@primary.*\n.*crdb_rewrite_inline_hints.*\@xy161829.*\n.*crdb_rewrite_inline_hints.*\@y161829`)
1211+
if reg.FindString(contents) == "" {
1212+
return errors.Errorf("could not find full crdb_rewrite_inline_hints in schema.sql")
1213+
}
1214+
}
1215+
return nil
1216+
}, false, /* expectErrors */
1217+
base, plans, "distsql.html vec.txt vec-v.txt stats-defaultdb.public.table161829.sql",
1218+
)
1219+
1220+
// Test that the stmt argument must have its single quotes escaped.
1221+
r.Exec(t, `SELECT information_schema.crdb_rewrite_inline_hints('SELECT * FROM table161829 WHERE z = ''hello''', 'SELECT * FROM table161829@xz161829 WHERE z = ''hello''')`)
1222+
rows = r.QueryStr(t, "EXPLAIN ANALYZE (DEBUG) SELECT * FROM table161829 WHERE z = 'hello'")
1223+
checkBundle(
1224+
t, fmt.Sprint(rows), "table161829", func(name, contents string) error {
1225+
if name == "schema.sql" {
1226+
reg := regexp.MustCompile(`crdb_rewrite_inline_hints\(e\'SELECT \* FROM table161829 WHERE z = \\'hello\\'\'`)
1227+
if reg.FindString(contents) == "" {
1228+
return errors.Errorf("could not find full crdb_rewrite_inline_hints in schema.sql")
1229+
}
1230+
}
1231+
return nil
1232+
}, false, /* expectErrors */
1233+
base, plans, "distsql.html vec.txt vec-v.txt stats-defaultdb.public.table161829.sql",
1234+
)
1235+
})
1236+
11811237
// TODO(yuzefovich): figure out why this test occasionally fails under
11821238
// stress (i.e. it seems that no bundle is collected altogether).
11831239
//t.Run("under different users", func(t *testing.T) {

pkg/sql/hintpb/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ go_library(
2626
importpath = "github.com/cockroachdb/cockroach/pkg/sql/hintpb",
2727
visibility = ["//visibility:public"],
2828
deps = [
29+
"//pkg/sql/lexbase",
2930
"//pkg/sql/protoreflect",
3031
"//pkg/util/json",
3132
"//pkg/util/protoutil",

pkg/sql/hintpb/statement_hint.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
package hintpb
77

88
import (
9+
"fmt"
10+
11+
"github.com/cockroachdb/cockroach/pkg/sql/lexbase"
912
"github.com/cockroachdb/cockroach/pkg/sql/protoreflect"
1013
"github.com/cockroachdb/cockroach/pkg/util/json"
1114
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -44,6 +47,21 @@ func (h StatementHintUnion) HintType() string {
4447
}
4548
}
4649

50+
// RecreateStmt returns the SQL statement that can be used to recreate the hint.
51+
// Returns the empty string and false if the hint type is not supported.
52+
func (h StatementHintUnion) RecreateStmt(stmt string) (string, bool) {
53+
switch t := h.GetValue().(type) {
54+
case *InjectHints:
55+
return fmt.Sprintf(
56+
"SELECT information_schema.crdb_rewrite_inline_hints(%s, %s);",
57+
lexbase.EscapeSQLString(stmt),
58+
lexbase.EscapeSQLString(t.DonorSQL),
59+
), true
60+
default:
61+
return "", false
62+
}
63+
}
64+
4765
// Details returns a JSON representation of the hint details. This is used for
4866
// displaying hint information in SHOW STATEMENT HINTS WITH DETAILS.
4967
func (h *StatementHintUnion) Details() (json.JSON, error) {

pkg/sql/opt/exec/execbuilder/testdata/system

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,52 @@ distribution: local
173173
table: table_statistics_locks@primary
174174
spans: [/0/1 - /0/1]
175175
locking strength: for update
176+
177+
# Query used by the stmt bundle to fetch hints for recreation.
178+
query T
179+
EXPLAIN (OPT, VERBOSE) SELECT hint FROM system.statement_hints WHERE fingerprint = 'SELECT * FROM t WHERE x = _' ORDER BY created_at
180+
----
181+
distribute
182+
├── columns: hint:4 [hidden: created_at:5]
183+
├── stats: [rows=10]
184+
├── cost: 291.694386
185+
├── cost-flags: unbounded-cardinality
186+
├── ordering: +5
187+
├── distribution: test
188+
├── input distribution:
189+
├── prune: (4,5)
190+
└── sort
191+
├── columns: hint:4 created_at:5
192+
├── stats: [rows=10]
193+
├── cost: 91.6743864
194+
├── cost-flags: unbounded-cardinality
195+
├── ordering: +5
196+
├── prune: (4,5)
197+
└── project
198+
├── columns: hint:4 created_at:5
199+
├── stats: [rows=10]
200+
├── cost: 90.5900007
201+
├── cost-flags: unbounded-cardinality
202+
├── prune: (4,5)
203+
└── select
204+
├── columns: fingerprint:3 hint:4 created_at:5
205+
├── stats: [rows=10, distinct(3)=1, null(3)=0]
206+
├── cost: 90.4700007
207+
├── cost-flags: unbounded-cardinality
208+
├── fd: ()-->(3)
209+
├── index-join statement_hints
210+
│ ├── columns: fingerprint:3 hint:4 created_at:5
211+
│ ├── stats: [rows=10]
212+
│ ├── cost: 90.3400007
213+
│ ├── cost-flags: unbounded-cardinality
214+
│ ├── interesting orderings: (+1)
215+
│ └── scan statement_hints@hash_idx
216+
│ ├── columns: row_id:1
217+
│ ├── constraint: /2/1: [/6457443302007961978 - /6457443302007961978]
218+
│ ├── stats: [rows=10, distinct(2)=1, null(2)=0]
219+
│ ├── cost: 28.6200001
220+
│ ├── cost-flags: unbounded-cardinality
221+
│ ├── key: (1)
222+
│ └── interesting orderings: (+1)
223+
└── filters
224+
└── fingerprint:3 = 'SELECT * FROM t WHERE x = _' [outer=(3), constraints=(/3: [/'SELECT * FROM t WHERE x = _' - /'SELECT * FROM t WHERE x = _']; tight), fd=()-->(3)]

pkg/sql/parserutils/utils.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,22 @@ var ParseQualifiedTableName = func(sql string) (*tree.TableName, error) {
105105
var PLpgSQLParse = func(sql string) (statements.PLpgStatement, error) {
106106
return statements.PLpgStatement{}, errors.AssertionFailedf("sql.DoParserInjection hasn't been called")
107107
}
108+
109+
// FingerprintStatement parses a SQL string and converts it to a statement
110+
// fingerprint. If the input is already a fingerprint, it is returned as-is.
111+
// The optional extraFlags are OR'd into the default FmtHideConstants flags used
112+
// for fingerprinting.
113+
func FingerprintStatement(sql string, extraFlags ...tree.FmtFlags) (string, error) {
114+
stmts, err := Parse(sql)
115+
if err != nil {
116+
return "", pgerror.Wrapf(
117+
err, pgcode.InvalidParameterValue, "could not parse statement",
118+
)
119+
}
120+
if len(stmts) != 1 {
121+
return "", pgerror.Newf(
122+
pgcode.InvalidParameterValue, "could not parse as a single SQL statement",
123+
)
124+
}
125+
return tree.FormatStatementHideConstants(stmts[0].AST, extraFlags...), nil
126+
}

0 commit comments

Comments
 (0)