sql: stub implementation of EXPLAIN ANALYZE (DEBUG, REDACT)

[michae2]

sql: stub implementation of EXPLAIN ANALYZE (DEBUG, REDACT)

Add a new explain flag, REDACT, which can be used to collect a
redacted statement bundle with EXPLAIN ANALYZE (DEBUG, REDACT).
Initially this is the only variant of EXPLAIN that supports REDACT
but the possibility of other variants using REDACT is left open.

This first commit plumbs the redact flag into explain_bundle.go but does
not implement redaction for any of the files, instead simply omitting
files which could contain user data. Following commits will add
redaction support for each file.

Part of: #68570

Epic: CRDB-19756

Release note (sql change): Add a new REDACT flag to EXPLAIN which
causes constants, literal values, parameter values, and any other user
data to be redacted in explain output. Redacted statement diagnostics
bundles can now be collected with EXPLAIN ANALYZE (DEBUG, REDACT).

sql: add statement.sql to EXPLAIN ANALYZE (DEBUG, REDACT)

Support redaction of statement.sql, and add it back to redacted
statement diagnostics bundles.

Part of: #68570

Epic: CRDB-19756

Release note: None

[cockroach-teamcity]

This change is 

[michae2]

• RFC draft is in docs/RFCS: redact statement diagnostics bundles #92306 (not necessary to read, but has a little background).
• More commits are coming, but these first two are RFAL.

[michae2]

@RaduBerinde said he would take a look, so dropping @rytaft (unless you really want to).

[RaduBerinde]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @yuzefovich)

---

pkg/sql/explain_bundle.go line 226 at r2 (raw file):

// placeholder arguments).
func (b *stmtBundleBuilder) buildPrettyStatement() {
	if b.flags.RedactValues && b.stmt != "" {

What is the rationale for this block and the != "" check? (maybe add a comment)

---

pkg/sql/explain_bundle.go line 242 at r2 (raw file):

		cfg.ValueRedaction = b.flags.RedactValues
		b.stmt = cfg.Pretty(b.plan.stmt.AST)
		if b.flags.RedactValues {

Please add a comment explaining why the two steps are needed. From looking around for a bit, it looks like Pretty will enclose the sensitive values with markers and then Redact actually hides them?

[michae2]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RaduBerinde and @yuzefovich)

---

pkg/sql/explain_bundle.go line 226 at r2 (raw file):

Previously, RaduBerinde wrote…

What is the rationale for this block and the != "" check? (maybe add a comment)

If we don't have the AST for some reason, we're now falling back to using the raw SQL. (This was added recently in #94440.) This block redacts that raw SQL. But if the raw SQL is empty (i.e. we don't have it for some reason) then it seems better to write out -- no statement than ‹×›, so the != "" check ensures that.

This part was awkward so I rewrote it a little, LMK what you think.

---

pkg/sql/explain_bundle.go line 242 at r2 (raw file):

Previously, RaduBerinde wrote…

Please add a comment explaining why the two steps are needed. From looking around for a bit, it looks like Pretty will enclose the sensitive values with markers and then Redact actually hides them?

Yes, that's right. For constants, Pretty is calling formatNodeMaybeMarkRedaction which only adds redaction markers, so then we have to call Redact to actually hide.

(If I ever get around to implementing EXPLAIN ANALYZE (DEBUG, REDACTABLE) then the Pretty output will be the desired output, so it seemed ok to me for REDACT to have this second step.)

[RaduBerinde]

Reviewed 4 of 6 files at r1, 2 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @yuzefovich)

[michae2]

TFYR! I'll wait to rebase on #94949 before borsing.

[yuzefovich]

What is the rationale for backporting this? Is it a part of a customer commit or something like that?

Reviewed 6 of 6 files at r1, 2 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @RaduBerinde)

[michae2]

Rebased on #94949.

What is the rationale for backporting this? Is it a part of a customer commit or something like that?

This is needed for PCI DSS compliance, which I believe is targeted for a minor version of v22.2 sometime around the end of this month (is that right, @abarganier?).

Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @abarganier, @RaduBerinde, and @yuzefovich)

[michae2]

Merging this, and I'll continue the work in #95136. TFYRs!

bors r=RaduBerinde,yuzefovich

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from a100707 to blathers/backport-release-22.2-94950: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.x failed. See errors above.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}