Skip to content

Comments

Implement SHOW REDACTED CREATE ...#31887

Merged
ggevay merged 3 commits intoMaterializeInc:mainfrom
ggevay:show-redacted-create
Mar 17, 2025
Merged

Implement SHOW REDACTED CREATE ...#31887
ggevay merged 3 commits intoMaterializeInc:mainfrom
ggevay:show-redacted-create

Conversation

@ggevay
Copy link
Contributor

@ggevay ggevay commented Mar 14, 2025

Add redacted counterparts to various SHOW CREATE ... statements. We need this for redaction in the self-managed debug tool (through mzexplore).

The first commit just tweaks some comments, while the second commit is the actual thing.

Motivation

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@ggevay ggevay added the A-ADAPTER Topics related to the ADAPTER layer label Mar 14, 2025
@ggevay ggevay force-pushed the show-redacted-create branch from b41bff5 to 3f783bc Compare March 14, 2025 18:04
@ggevay ggevay added the self-managed-backport-v25.1 Needs to be backported into the v25.1 self-managed release label Mar 14, 2025
@ggevay ggevay marked this pull request as ready for review March 14, 2025 18:37
@ggevay ggevay requested review from a team as code owners March 14, 2025 18:37
@ggevay ggevay requested a review from ParkMyCar March 14, 2025 18:37
@ggevay
Copy link
Contributor Author

ggevay commented Mar 15, 2025

Nightly: https://buildkite.com/materialize/nightly/builds/11468

  • The failure in "Unused dependencies" is unrelated.
  • The benchmark fail can't be related, as far as I can see.
  • The testdrive fail is probably also unrelated. Discussing here.

Copy link
Contributor

@ParkMyCar ParkMyCar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See one comment in the parser, but otherwise LGTM!

Comment on lines 7723 to 7731
let redacted = self.parse_keyword(REDACTED);
if self.parse_one_of_keywords(&[COLUMNS, FIELDS]).is_some() {
if redacted {
return parser_err!(
self,
self.peek_prev_pos(),
"SHOW REDACTED with COLUMNS or FIELD is not supported"
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using a bunch of if-statements, what if we did something like:

let redacted = self.parse_keyword(REDACTED);
if !self.peek_keyword(CREATE) {
  // ...return an error
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, thank you!

@ggevay ggevay force-pushed the show-redacted-create branch from 3f783bc to ceb8012 Compare March 17, 2025 16:38
@ggevay
Copy link
Contributor Author

ggevay commented Mar 17, 2025

Thanks for the review!

@ggevay ggevay enabled auto-merge March 17, 2025 16:38

```sql
SHOW CREATE CONNECTION <connection_name>
SHOW [REDACTED] CREATE CONNECTION <connection_name>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a blurb on what [REDACTED] does?
We can single source that statement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a patch with the description of the REDACTED option. Feel free to update the description in the show_create_redacted_option.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you very much @kay-kim !

@ggevay ggevay disabled auto-merge March 17, 2025 17:11
@ggevay ggevay enabled auto-merge March 17, 2025 20:08
@ggevay ggevay merged commit 6cf4a37 into MaterializeInc:main Mar 17, 2025
84 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ADAPTER Topics related to the ADAPTER layer self-managed-backport-v25.1-done self-managed-backport-v25.1 Needs to be backported into the v25.1 self-managed release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants