Skip to content

Commit

Permalink
sql: fix cached gists panic'ing after schema changes
Browse files Browse the repository at this point in the history
Previously we didn't guard against out of bounds column ids in gists,
if a gist is created on a table and then that table drops columns we
would hit a runtime index out of bounds panic. By design gist decoding
is best effort and should silently ignore these columns when this
happens. The columns are used for equality conditions, ie:

             └── • lookup join
                 │ table: broker@broker_pkey
                 │ equality: (tr_s_symb) = (b_id)

So they are a nice to have. Doing anything more sophisticated would
dramatically increase the gist size so isn't worth it.

Fixes: cockroachdb#76800

Release note: None
  • Loading branch information
cucaroach authored and maryliag committed Feb 28, 2022
1 parent 7052805 commit f9d727d
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 7 deletions.
22 changes: 22 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/explain_gist
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,25 @@ query T
SELECT crdb_internal.decode_plan_gist('$gist')
----
• <unknown>

# Regression test for #76800
statement ok
CREATE TABLE t2 (a int, b int, c int, d int, e int)

let $gist
EXPLAIN (GIST) SELECT * FROM t2

# To hit bug requires deleting lots of columns because of hidden columns.
statement ok
ALTER TABLE t2 DROP COLUMN b;
ALTER TABLE t2 DROP COLUMN c;
ALTER TABLE t2 DROP COLUMN a;
ALTER TABLE t2 DROP COLUMN d;
ALTER TABLE t2 DROP COLUMN e

query T
SELECT crdb_internal.decode_plan_gist('$gist')
----
• scan
table: t2@t2_pkey
spans: FULL SCAN
14 changes: 9 additions & 5 deletions pkg/sql/opt/exec/explain/result_columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,15 @@ func getResultColumns(
func tableColumns(table cat.Table, ordinals exec.TableColumnOrdinalSet) colinfo.ResultColumns {
cols := make(colinfo.ResultColumns, 0, ordinals.Len())
for i, ok := ordinals.Next(0); ok; i, ok = ordinals.Next(i + 1) {
col := table.Column(i)
cols = append(cols, colinfo.ResultColumn{
Name: string(col.ColName()),
Typ: col.DatumType(),
})
// Be defensive about bitset values because they may come from cached
// gists and the columns they refer to could have been removed.
if i < table.ColumnCount() {
col := table.Column(i)
cols = append(cols, colinfo.ResultColumn{
Name: string(col.ColName()),
Typ: col.DatumType(),
})
}
}
return cols
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/plan_opt.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,8 +655,7 @@ func (opc *optPlanningCtx) runExecBuilder(
return nil
}

// DecodeGist Avoid an import cycle by keeping the cat out of the tree, RFC: is
// there a better way?
// DecodeGist Avoid an import cycle by keeping the cat out of the tree.
func (p *planner) DecodeGist(gist string) ([]string, error) {
return explain.DecodePlanGistToRows(gist, &p.optPlanningCtx.catalog)
}
Expand Down

0 comments on commit f9d727d

Please sign in to comment.