sql: add DDL support for CREATE/DROP TABLE in stored procedures#168464
Conversation
|
😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details. |
|
Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link) |
859a024 to
d832d9d
Compare
fqazi
left a comment
There was a problem hiding this comment.
@spilchen Looks good and straight forward.
@fqazi reviewed 11 files and all commit messages, and made 3 comments.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on michae2 and spilchen).
pkg/sql/opt/optbuilder/builder.go line 390 at r1 (raw file):
case *tree.DoBlock: case *tree.CreateTable, *tree.DropTable: // DDL is allowed inside stored procedures when the cluster has
We probably wan to move this comment down.
pkg/sql/logictest/testdata/logic_test/procedure_ddl line 795 at r1 (raw file):
# # TODO(spilchen): this will be something we need to address. We must upgrade to # SERIALIZABLE for DDL in procedures to ensure safety.
Yeah, this would impact other schema changes
michae2
left a comment
There was a problem hiding this comment.
Wow, didn't realize that's (pretty much) all it would take!
@michae2 reviewed 13 files and all commit messages, and made 2 comments.
Reviewable status:complete! 2 of 0 LGTMs obtained (waiting on fqazi and spilchen).
pkg/sql/logictest/testdata/logic_test/procedure_ddl line 795 at r1 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
Yeah, this would impact other schema changes
+1
Allow CREATE TABLE and DROP TABLE inside PL/pgSQL stored procedure bodies, gated by the cluster version (v26.3). This is a lightweight temporary gate — it blocks the feature in mixed-version clusters and auto-enables once the upgrade to v26.3 is finalized. The optbuilder allowlist is extended to permit these two DDL statements in procedure definitions (not functions or DO blocks). At compile time, all statement references are resolved against the current catalog, so DDL followed by DML on the newly created table in the same procedure body fails — a known divergence from PostgreSQL's late-binding model. The logic test exercises: transaction commit/rollback, exception handler savepoints, nested procedures, autocommit_before_ddl interaction, compile-time binding behavior, and weak isolation levels (READ COMMITTED, REPEATABLE READ). Closes cockroachdb#168460 Epic: none Release note: None Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d832d9d to
417c33f
Compare
spilchen
left a comment
There was a problem hiding this comment.
@spilchen made 1 comment.
Reviewable status:complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on fqazi and michae2).
pkg/sql/opt/optbuilder/builder.go line 390 at r1 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
We probably wan to move this comment down.
Done.
|
TFTRs! /trunk merge |
Allow CREATE TABLE and DROP TABLE inside PL/pgSQL stored procedurebodies, gated by the cluster version (v26.3). This is a lightweight temporary gate — it blocks the feature in mixed-version clusters and
auto-enables once the upgrade to v26.3 is finalized.
The optbuilder allowlist is extended to permit these two DDL statements in procedure definitions (not functions or DO blocks). At compile time, all statement references are resolved against the current catalog, so DDL followed by DML on the newly created table in the same procedure body fails. This is a known divergence from PostgreSQL's late-binding model and will be changed later.
Closes #168460
Epic: CRDB-31256
Release note: None