REDACT option for EXPLAIN, EXPLAIN ANALYZE #16929
Conversation
Files changed: |
|
If you want a statement that shows off a little more redaction, maybe one with constants in a few more places, like: EXPLAIN (REDACT) SELECT (SELECT sum(revenue / 2.0) FROM rides r2 WHERE r2.rider_id = rider_id) AS rsum FROM rides WHERE end_time - start_time <= '1 day' AND city IN ('los angeles', 'san francisco');But maybe this is overkill... I think the statement you have is fine. |
|
Thanks @michae2 - I'll stick with the statement I used, in that case. How does this rest of this PR look to you? |
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
This is looking great, just some minor comments. Thanks for doing this!
Reviewed all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @taroface)
v23.1/explain.md line 456 at r3 (raw file):
- The types of the values used in the statement plan. - The SQL expressions that were involved in each processing stage, and includes the columns used by each level.
If you wanted to, you could mention that using the TYPES option also includes everything from VERBOSE. (And this is also true for (OPT, TYPES) and (DISTSQL, TYPES), which include everything from (OPT, VERBOSE) and (DISTSQL, VERBOSE), respectively.)
(Not sure how important this is, might be too esoteric.)
v23.1/explain.md line 513 at r3 (raw file):
~~~ `OPT` has five suboptions: [`VERBOSE`](#opt-verbose-option), [`TYPES`](#opt-types-option), [`ENV`](#opt-env-option), [`MEMO`](#opt-memo-option), [`REDACT`](#opt-redact-option).
There's also CATALOG but seems like you're intentionally omitting this one (if so, please ignore this comment...)
v23.1/explain.md line 776 at r3 (raw file):
~~~ ##### `OPT, REDACT` option
REDACT is similar to VERBOSE and TYPES in that it works for both normal EXPLAIN (a.k.a. EXPLAIN (PLAN)) and also EXPLAIN (OPT). So in the same way that there are separate headings for VERBOSE and OPT, VERBOSE, I think it would be consistent to have separate headings for REDACT and OPT, REDACT.
v23.1/explain.md line 780 at r3 (raw file):
The `REDACT` suboption causes constants, literal values, parameter values, and personally identifiable information (PII) to be redacted as `‹×›` in the physical statement plan. You can also use the `REDACT` option in combination with the [`VERBOSE`](#opt-verbose-option), [`TYPES`](#opt-types-option), and [`MEMO`](#opt-memo-option) suboptions.
REDACT also works with CATALOG as well. (Oh, seems like you're intentionally omitting OPT, CATALOG? In which case, never mind).
|
This change is |
There was a problem hiding this comment.
Thank you! PTAL.
Reviewable status:
v23.1/explain.md line 456 at r3 (raw file):
Previously, michae2 (Michael Erickson) wrote…
If you wanted to, you could mention that using the
TYPESoption also includes everything fromVERBOSE. (And this is also true for(OPT, TYPES)and(DISTSQL, TYPES), which include everything from(OPT, VERBOSE)and(DISTSQL, VERBOSE), respectively.)(Not sure how important this is, might be too esoteric.)
Done.
v23.1/explain.md line 513 at r3 (raw file):
Previously, michae2 (Michael Erickson) wrote…
There's also
CATALOGbut seems like you're intentionally omitting this one (if so, please ignore this comment...)
Yes, this hasn't previously been documented for some reason. I'll have to do some digging, but I'm leaving it out of this PR for expediency.
v23.1/explain.md line 776 at r3 (raw file):
Previously, michae2 (Michael Erickson) wrote…
REDACTis similar toVERBOSEandTYPESin that it works for both normalEXPLAIN(a.k.a.EXPLAIN (PLAN)) and alsoEXPLAIN (OPT). So in the same way that there are separate headings forVERBOSEandOPT, VERBOSE, I think it would be consistent to have separate headings forREDACTandOPT, REDACT.
Done. I did not actually realize REDACT worked without OPT.
v23.1/explain.md line 780 at r3 (raw file):
Previously, michae2 (Michael Erickson) wrote…
REDACTalso works withCATALOGas well. (Oh, seems like you're intentionally omittingOPT, CATALOG? In which case, never mind).
Yep, see earlier comment.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status:
103077: docgen: add EXPLAIN, EXPLAIN ANALYZE options to diagrams r=michae2 a=taroface Add various `EXPLAIN` and `EXPLAIN ANALYZE` options that were documented but not exposed in the SQL diagrams. Also add `REDACT`, which will be documented with cockroachdb/docs#16929. Epic: none Release note: none Release justification: non-production code change 103083: kvserver: deflake TestPromoteNonVoterInAddVoter r=andrewbaptist a=kvoli It is possible for span config updates to arrive at different times between stores. `TestPromoteNonVoterInAddVoter` was flaking when the incoming leaseholder would act upon a stale span config, before receiving the updated one which the outgoing leaseholder used. This resulted in the test failing as more than just the two expected promotion events appeared in the range log, as the incoming leaseholder removed voters, then subsequently added them back upon receiving the up to date span configuration. #103086 tracks this issue. This PR checks on the prefix of the range log add voter events, to avoid failing the test when this untimely behavior occurs. Stressed overnight, removing the skip under stress flag: ``` dev test pkg/kv/kvserver -f TestPromoteNonVoterInAddVoter -v --stress --stress-args="-p 4" ... 27158 runs so far, 0 failures, over 12h56m55s ``` This PR also adds additional (v=6) logging of the range descriptor and span config, as these come in handy when debugging failures such as this. Fixes: #101519 Release note: None 103332: cloud/kubernetes: bump version to v23.1.0 r=ZhouXing19 a=ZhouXing19 Epic: None Release note: None 103335: util/version: update roachtest version for 23.2 to 23.1.0 r=ZhouXing19 a=ZhouXing19 Epic: None Release note: None 103338: ci: additionally build `workload` and `dev` for all Unix systems r=rail,srosenberg a=rickystewart This includes ARM machines and macOS systems. Epic: none Release note: None Co-authored-by: Ryan Kuo <[email protected]> Co-authored-by: Austen McClernon <[email protected]> Co-authored-by: Jane Xing <[email protected]> Co-authored-by: Ricky Stewart <[email protected]>
DOC-6758
REDACToption forEXPLAINandEXPLAIN ANALYZE.@michae2, I'm not sure if the example statements I used were ideal for showing redacted values. If you can think of a better one, please feel free to suggest one. It would have to use the MoVR dataset for consistency with the other docs examples.