Skip to content

Commit

Permalink
sql: fix gist decoding in foreign DB
Browse files Browse the repository at this point in the history
When a gist is exported and decoded in a foreign DB it should still
decode w/o panic. If a table scan result columns are fed into a
group by column the group by column code expects the result columns to
be in tact and panics. Fix this by making fake "unknown" result columns
so we can print the plan.

Making this change led to an unintended change in gists_tpce, this is
because the catalog was shared between the test files and I added a
table.  Fix this for the future by creating a new catalog for each file.
But with this change the table ids encoded in the gists_tpce got
smaller.

Epic: None
Fixes: #110964
Release note (bug fix): Fixed panic when decoding gist in DB without the
table referred to by the gist.
  • Loading branch information
cucaroach committed Sep 20, 2023
1 parent a499165 commit e5b79ee
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 3 deletions.
2 changes: 2 additions & 0 deletions pkg/sql/opt/exec/explain/plan_gist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ func TestPlanGistBuilder(t *testing.T) {
}
// RFC: should I move this to opt_tester?
datadriven.RunTest(t, datapathutils.TestDataPath(t, "gists"), testGists)
// Reset the catalog for the next test.
catalog = testcat.New()
datadriven.RunTest(t, datapathutils.TestDataPath(t, "gists_tpce"), testGists)
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/opt/exec/explain/result_columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
package explain

import (
"fmt"

"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
Expand Down Expand Up @@ -255,6 +257,12 @@ func tableColumns(table cat.Table, ordinals exec.TableColumnOrdinalSet) colinfo.
Name: string(col.ColName()),
Typ: col.DatumType(),
})
} else {
// Give downstream operators something to chew on so that they don't panic.
cols = append(cols, colinfo.ResultColumn{
Name: fmt.Sprintf("unknownCol-%d", i),
Typ: types.Unknown,
})
}
}
return cols
Expand Down
44 changes: 44 additions & 0 deletions pkg/sql/opt/exec/explain/testdata/gists
Original file line number Diff line number Diff line change
Expand Up @@ -1122,3 +1122,47 @@ explain(shape):
syntax: "select 123"
explain(gist):
• show completions

exec-ddl
CREATE TABLE ts (a int, b STRING)
----

gist-explain-roundtrip
SELECT max(a), b FROM ts WHERE a = 1 AND b LIKE '$ internal%' GROUP BY b
----
hash: 6959966538333657501
plan-gist: AgF0AgADAAAAAwsCBgQ=
explain(shape):
• group (hash)
│ group by: b
└── • filter
│ filter: (a = _) AND (b LIKE _)
└── • scan
table: ts@ts_pkey
spans: FULL SCAN
explain(gist):
• group (hash)
│ group by: a
└── • filter
└── • scan
table: ts@ts_pkey
spans: FULL SCAN

# The above query produces this gist when run via EXPLAIN (GIST) in a real DB and when
# run in a DB w/o that table panics, this tests that the panic is fixed. Essentially we're
# exercising the "unknown" table code.
explain-plan-gist
AgHQAQIAAwAAAAMLAgYE
----
• group (hash)
│ group by: unknownCol-0
└── • filter
└── • scan
table: ?@?
spans: FULL SCAN
6 changes: 3 additions & 3 deletions pkg/sql/opt/exec/explain/testdata/gists_tpce
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ GROUP BY b_name
ORDER BY 2 DESC
----
hash: 14732118561700026222
plan-gist: AgGUAQQAPAAAAAGqAQQAAwIAABQAogEEAgAUAJgBBAIAFACsAQQCAAkAAgIAARQAhgECAgEHBAsCBwQRBgQ=
plan-gist: AgGKAQQAPAAAAAGgAQQAAwIAABQAmAEEAgAUAI4BBAIAFACiAQQCAAkAAgIAARQAfAICAQcECwIHBBEGBA==
explain(shape):
• sort
│ order: -sum
Expand Down Expand Up @@ -116,7 +116,7 @@ LEFT JOIN last_trade ON lt_s_symb = hs_s_symb
LIMIT 10;
----
hash: 10378570089558454947
plan-gist: AgF4BAAFAgAAE3gCFAGAAQICABQBpAECAgEHBgsCBwYYBgY=
plan-gist: AgFuBAAFAgAAE24CFAF2AgIAFAGaAQICAQcGCwIHBhgGBg==
explain(shape):
• top-k
│ order: +"coalesce"
Expand Down Expand Up @@ -211,7 +211,7 @@ update_trade_submitted AS (
SELECT * FROM request_list;
----
hash: 7096273538769246907
plan-gist: AgGkAQIAHwIAAAcQBRAhpAEAAAcCMAGUAQIAHwAAAAMHCDAxBQIUAJQBAgIBBQgHCAUII5QBAAcCMDEFAgcGBQYwH5IBADEFAhQFkAECAgEqMQUCFAWwAQICASoHAjAxBQIUAJABAgIBBRwHIAUgMCGQAQAAMQUCFAWwAQICASoHAjAxBQgGCA==
plan-gist: AgGaAQIAHwIAAAcQBRAhmgEAAAcCMAGKAQIAHwAAAAMHCDAxBQIUAIoBAgIBBQgHCAUII4oBAAcCMDEFAgcGBQYwH4gBADEFAhQFhgECAgEqMQUCFAWmAQICASoHAjAxBQIUAIYBAgIBBRwHIAUgMCGGAQAAMQUCFAWmAQICASoHAjAxBQgGCA==
explain(shape):
• root
Expand Down

0 comments on commit e5b79ee

Please sign in to comment.