Skip to content

Commit 3180fdf

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 3180fdf

File tree

3 files changed

+144
-0
lines changed

3 files changed

+144
-0
lines changed

pkg/sql/explain_bundle.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
2929
"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain"
3030
"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
31+
"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
3132
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
3233
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
3334
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -556,6 +557,68 @@ var stmtBundleIncludeAllFKReferences = settings.RegisterBoolSetting(
556557
false,
557558
)
558559

560+
// getDonorSqls returns the string representation of all the donor sql statements
561+
// of the target statement. The returned donor sqls should have been sorted in
562+
// ascending order by their creation time.
563+
func (c *stmtEnvCollector) getDonorSqls(
564+
ctx context.Context, stmt string, sv *settings.Values,
565+
) (donorSqls []string, err error) {
566+
fingerprint, err := parserutils.FingerprintStatement(stmt,
567+
tree.FmtFlags(tree.QueryFormattingForFingerprintsMask.Get(
568+
sv,
569+
)))
570+
if err != nil {
571+
return donorSqls, err
572+
}
573+
574+
query := `SELECT crdb_internal.pb_to_json(
575+
'hintpb.StatementHintUnion', hint)->'injectHints'->>'donorSql'
576+
AS donor_sql FROM system.statement_hints WHERE fingerprint = $1
577+
ORDER BY "created_at"`
578+
579+
rows, err := c.ie.QueryBufferedEx(
580+
ctx,
581+
"stmtEnvCollector",
582+
nil, /* txn */
583+
c.ieo,
584+
query,
585+
fingerprint,
586+
)
587+
if err != nil {
588+
return donorSqls, err
589+
}
590+
591+
donorSqls = make([]string, 0, len(rows))
592+
for _, row := range rows {
593+
if len(row) == 0 {
594+
return nil, errors.AssertionFailedf("expect not nil row")
595+
}
596+
if row[0] == tree.DNull {
597+
continue
598+
}
599+
strDatum, ok := row[0].(*tree.DString)
600+
if !ok {
601+
return nil, errors.AssertionFailedf("expect row to be a string, got %T", row[0])
602+
}
603+
donorSqls = append(donorSqls, strDatum.String())
604+
}
605+
return donorSqls, nil
606+
}
607+
608+
func (c *stmtEnvCollector) PrintStmtHints(
609+
ctx context.Context, stmt string, sv *settings.Values, w io.Writer,
610+
) error {
611+
donorSqls, err := c.getDonorSqls(ctx, stmt, sv)
612+
if err != nil {
613+
return err
614+
}
615+
for _, donorSql := range donorSqls {
616+
// donorSql has been quoted. Escape stmt as well to handle embedded quotes.
617+
fmt.Fprintf(w, "SELECT information_schema.crdb_rewrite_inline_hints(%s, %s);\n", lexbase.EscapeSQLString(stmt), donorSql)
618+
}
619+
return nil
620+
}
621+
559622
// stmtBundleStatsFileRE is a regex that matches all "complex" characters that
560623
// might not be safe when used in file names on some systems.
561624
var stmtBundleStatsFileRE = regexp.MustCompile(`[^a-zA-Z0-9_.\-]`)
@@ -1007,6 +1070,13 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) {
10071070
b.printError(fmt.Sprintf("-- error getting schema for view %s: %v", views[i].FQString(), err), &buf)
10081071
}
10091072
}
1073+
1074+
if !b.flags.RedactValues {
1075+
if err := c.PrintStmtHints(ctx, b.stmt, b.sv, &buf); err != nil {
1076+
b.printError(fmt.Sprintf("-- error getting statement hints: %v", err), &buf)
1077+
}
1078+
}
1079+
10101080
if buf.Len() == 0 {
10111081
buf.WriteString("-- there were no objects used in this query\n")
10121082
}

pkg/sql/explain_bundle_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,6 +1178,61 @@ CREATE TABLE users(id UUID DEFAULT gen_random_uuid() PRIMARY KEY, promo_id INT R
11781178
)
11791179
})
11801180

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

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)