sql: fix crash when explaining a constrained virtual table scan #58208
Conversation
|
This change is |
RaduBerinde
force-pushed
the
explain-spans-crash
branch
from
fe3efa3 to
e5cff51
Compare
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis and @RaduBerinde)
pkg/sql/explain_plan.go, line 171 at r1 (raw file):
spanFormatFn := func(table cat.Table, index cat.Index, scanParams exec.ScanParams) string { if table.IsVirtualTable() { return "<internal error>"
Feels like this message could have a bit more detail -- maybe mention virtual tables?
When emitting `EXPLAIN (VERBOSE)` output, we use a callback to generate and format the physical spans; this callback currently crashes for virtual tables. Note that this code path is used when collecting statement diagnostics, which can lead to a severe situation. The fix is to always show the logical spans and use the callback only for physical tables. In addition, we add logic to catch panics inside emitExplain; unfortunately, it would not have helped here because the panic object was a bare string (and not an assertion error). Fixes cockroachdb#58193. Release note (bug fix): fixed a "column family 0 not found" crash caused by explaining or gathering statement diagnostics on certain queries involving virtual tables.
RaduBerinde
force-pushed
the
explain-spans-crash
branch
from
e5cff51 to
d0b3e2d
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/explain_plan.go, line 171 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
Feels like this message could have a bit more detail -- maybe mention virtual tables?
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:
There was a problem hiding this comment.
bors r+
Reviewable status:
|
Build succeeded: |
When emitting
EXPLAIN (VERBOSE)output, we use a callback togenerate and format the physical spans; this callback currently
crashes for virtual tables.
Note that this code path is used when collecting statement
diagnostics, which can lead to a severe situation.
The fix is to always show the logical spans and use the callback only
for physical tables.
In addition, we add logic to catch panics inside emitExplain;
unfortunately, it would not have helped here because the panic object
was a bare string (and not an assertion error).
Fixes #58193.
Release note (bug fix): fixed a "column family 0 not found" crash
caused by explaining or gathering statement diagnostics on certain
queries involving virtual tables.