sem/builtins: vectorized engine no longer catches crdb_internal.force_panic #63178
Conversation
|
This change is |
…_panic Previously, when executing `crdb_internal.force_panic` builtin, if it was executed via the vectorized engine, we would catch the panic and convert it into an internal error instead. This is undesirable and is now fixed. Release note: None
yuzefovich
changed the title
|
This behavior was only introduced last week: #62889. |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis and @michae2)
There was a problem hiding this comment.
Reviewed 4 of 5 files at r1.
Reviewable status:
pkg/sql/sem/builtins/builtins.go, line 4085 at r1 (raw file):
// This code is unreachable. panic(msg)
Why leave it in, then? For some sort of static analysis?
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/sql/sem/builtins/builtins.go, line 4085 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
// This code is unreachable. panic(msg)Why leave it in, then? For some sort of static analysis?
The compiler complains - it cannot prove to itself that this line is unreachable, so it expects either Golang's panic or return ...; I chose the former. This fact is one of the annoyances of using colexecerror methods.
|
Build succeeded: |
Previously, when executing
crdb_internal.force_panicbuiltin, if itwas executed via the vectorized engine, we would catch the panic and
convert it into an internal error instead. This is undesirable and is
now fixed.
Release note: None