tree: correct mutation/DDL property for some opaque operators#93991
tree: correct mutation/DDL property for some opaque operators#93991craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
deb720e to
0cb4771
Compare
ZhouXing19
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @e-mbrown, @rafiss, and @stevendanna)
pkg/sql/tenant.go line 321 at r2 (raw file):
// CreateTenant implements the tree.TenantOperator interface. func (p *planner) CreateTenant(
Seems that tests for CreateTenant and DestroyTenant were not added.
|
Previously, ZhouXing19 (Jane Xing) wrote…
And |
In ed733ad, a framework was added to mark certain opaque operators as DDL or mutations. This was enhanced in 06581b3, but that change wasn't exhaustive since it marked some statements as read-only, even if they could perform DDL. With the addition of `StatementType()` in 8962176, we can make this a little more correct. This allows the check at https://github.com/cockroachdb/cockroach/blob/48ef0d89e6179c0d348a5236ad308d81fa392f7c/pkg/sql/opt/exec/execbuilder/relational.go#L163 to work correctly, and reject operations that shouldn't be allowed when using a read-only transaction. To explain each change: - BACKUP can modify job state and write to userfiles, so shouldn't be allowed in read-only mode. - SET commands are always allowed in read-only mode in order to match Postgres behavior, and since those changes are all in-memory and session setting modifications don't respect transactions anyway. - The crdb_internal tenant functions modify system tables. - GRANT, REVOKE, and many other privilege-related statements are "DCL" (data control language), and all modify system tables or descriptors. - Declaring cursors is allowed in Postgres read-only transactions. Release note (bug fix): CREATE ROLE, DELETE ROLE, GRANT, and REVOKE statements no longer work when the transaction is in read-only mode.
0cb4771 to
50a7999
Compare
|
tftr! i added tests bors r=ZhouXing19 |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 50a7999 to blathers/backport-release-22.1-93991: 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.1.x failed. See errors above. error creating merge commit from 50a7999 to blathers/backport-release-22.2-93991: 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. |
fixes #91713
In ed733ad, a framework was added to mark certain opaque operators as DDL or mutations.
This was enhanced in 06581b3, but that change wasn't exhaustive since it marked some statements as read-only, even if they could perform DDL.
With the addition of
StatementType()in8962176, we can make this a little more correct.
This allows the check at
cockroach/pkg/sql/opt/exec/execbuilder/relational.go
Line 163 in 48ef0d8
To explain each change:
Release note (bug fix): CREATE ROLE, DELETE ROLE, GRANT, and REVOKE statements no longer work when the transaction is in read-only mode.