opt: populate autocommit correctly under EXPLAIN #42500
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 13 of 13 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @justinj, @RaduBerinde, and @rytaft)
pkg/sql/opt/exec/execbuilder/testdata/autocommit, line 25 at r1 (raw file):
SET TRACING=OFF query B
Do you think it's worth keeping these checks as well? They aren't exactly redundant now.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/exec/execbuilder/testdata/autocommit, line 25 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Do you think it's worth keeping these checks as well? They aren't exactly redundant now.
I would if we didn't have the other tracing tests, but between those and the planning test these do seem redundant no?
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/exec/execbuilder/testdata/autocommit, line 25 at r1 (raw file):
Previously, RaduBerinde wrote…
I would if we didn't have the other tracing tests, but between those and the planning test these do seem redundant no?
Up to you. I'm fine with it either way.
|
bors r+ |
Build failed |
|
Hit #42280 bors r+ |
Build failed |
|
ui-test flake bors r+ |
|
bors r+ |
|
bors r- |
Canceled |
The autocommit decision was made on the entire query. This change modifies the behavior for EXPLAIN, where we want to pretend that the query under EXPLAIN is the entire query. We also add an `auto commit` field to the `EXPLAIN (VERBOSE)` output. We can now test the planning part of auto commit without using traces (the tests are updated accordingly). Release note (sql change): `EXPLAIN (VERBOSE)` now indicates if auto-commit will be used for mutations.
RaduBerinde
force-pushed
the
explain-autocommit
branch
from
26b52a4
to
378bbf0
Compare
|
bors r+ |
Build failed (retrying...) |
Build failed |
|
bors r+ |
Build failed |
|
Sigh.. Filed #42559 bors r+ |
Build failed |
|
bors r+ |
Build failed |
|
bors r+ |
42500: opt: populate autocommit correctly under EXPLAIN r=RaduBerinde a=RaduBerinde The autocommit decision was made on the entire query. This change modifies the behavior for EXPLAIN, where we want to pretend that the query under EXPLAIN is the entire query. We also add an `auto commit` field to the `EXPLAIN (VERBOSE)` output. We can now test the planning part of auto commit without using traces (the tests are updated accordingly). Release note: None Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Build succeeded |
The autocommit decision was made on the entire query. This change
modifies the behavior for EXPLAIN, where we want to pretend that the
query under EXPLAIN is the entire query.
We also add an
auto commitfield to theEXPLAIN (VERBOSE)output.We can now test the planning part of auto commit without using traces
(the tests are updated accordingly).
Release note: None