-
Notifications
You must be signed in to change notification settings - Fork 468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rbac: allow EXPLAIN
commands in user_privilege_hack
#21778
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, it might be worth it to add a test so that we don't accidentally break this in the future.
// * SHOW CREATE ... commands. See | ||
// <https://github.com/MaterializeInc/materialize/issues/18027> for | ||
// more details. | ||
// * EXPLAIN PLAN ... EXPLAIN TIMESTAMP and commands. See |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// * EXPLAIN PLAN ... EXPLAIN TIMESTAMP and commands. See | |
// * EXPLAIN PLAN ... and EXPLAIN TIMESTAMP ... commands. See |
ee0866d
to
5c87395
Compare
5c87395
to
a2b39c5
Compare
|
||
$ postgres-connect name=mz_support url=postgres://mz_support:materialize@${testdrive.materialize-internal-sql-addr} | ||
|
||
> CREATE SOURCE auction_house FROM LOAD GENERATOR AUCTION FOR ALL TABLES WITH (size = '1'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MaterializeInc/testing I just realized that having WITH (size = '1')
here might break CI (although it didn't break the CI tests for this PR). Feel free to remove this suffix.
Follow-up from #21760
Motivation
Turns out that one bit was missing from #21760 in order to resolve MaterializeInc/database-issues#6160.
This does in fact resolve MaterializeInc/database-issues#6160.
Tips for reviewer
I had to change the definition of
user_privilege_hack
a bit.Checklist
$T ⇔ Proto$T
mapping (possibly in a backwards-incompatible way), then it is tagged with aT-proto
label.