cockroachdb / cockroachdb/cockroach

sql/plpgsql: support dynamic DDL/DCL in EXECUTE

Open
#172,433 0 comments 0 reactions 0 assignees View on GitHub
A-sql-plpgsql A-sql-routine C-enhancement O-agent T-sql-foundations
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Is your feature request related to a problem? Please describe.**
CockroachDB's basic dynamic `EXECUTE` support (#172401) rejected any
schema-changing statement (DDL or DCL) in the dynamic command string with
a blanket check, even though the same statements are already allowed as
static SQL in stored procedure bodies via an allowlist. This blocked
common dynamic schema/partition-management and dynamic `GRANT`/`REVOKE`
patterns.

**Describe the solution you'd like**
- Dynamic `EXECUTE` runs the same DDL and DCL statements already allowed
as static SQL in stored procedure bodies: `CREATE`/`DROP TABLE`,
`CREATE`/`DROP SCHEMA`, `CREATE`/`DROP ROLE`, `GRANT`, `REVOKE`, and
`ALTER DEFAULT PRIVILEGES` (the `isAllowlistedProcedureDDL` set).
`checkDynamicExecuteStmtAllowed` already enforces the context rules
(transaction control rejected everywhere; schema-changing statements
only in procedures, only for the allowlisted set) — this only removes
the temporary blanket rejection sitting behind that check.
- The runtime machinery needs no changes, since it keys off the built
plan's DDL flag rather than static knowledge of the routine body:
- DDL in a procedure called from an explicit transaction is rejected
at execution via the existing `planFlagIsDDL` check.
- Buffered writes are disabled before a DDL body statement runs.
- Statements after the `EXECUTE` can reference the object it created:
the rest-of-body tail is always built at execution time, after the
dynamic statement has run, so it resolves the new object through the
transaction's uncommitted descriptors — this works even without the
late-binding cluster setting, since the tail is never analyzed at
`CREATE` time.

**Known limitation.** Under weak isolation, the pre-`CALL` isolation
upgrade (`maybeAutoCommitBeforeDDL`) walks the static body for DDL and
cannot see into a dynamic command string, so dynamic DDL falls through to
the in-routine runtime safety net and fails with a clear error instead of
auto-upgrading the transaction to `SERIALIZABLE` (as static DDL in a
procedure would). Treating every `EXECUTE` as potential DDL in that
walker would force an isolation upgrade on any `CALL` containing dynamic
SQL at all, which is worse than requiring the caller to already run at
`SERIALIZABLE` — the error message tells the user to do so. This is the
intended final behavior for this sliver, not an open gap.

**Describe alternatives you've considered**
Auto-upgrading isolation whenever a routine body contains any dynamic
`EXECUTE` (treating all dynamic SQL as potential DDL) was rejected — it
would force `SERIALIZABLE` on routines that never touch DDL, which is
worse than the current explicit error asking the caller to opt in.

**Additional context**
Part of the dynamic-SQL EXECUTE effort tracked in #169581. Builds on basic
`EXECUTE` support (#172401).

Epic CRDB-48117

Jira issue: CRDB-65669

Contributor guide

Open the contributing guide

Research direction

Start at checkDynamicExecuteStmtAllowed and the temporary blanket rejection behind it, then compare the permitted statements with the isAllowlistedProcedureDDL set. Verify that dynamic EXECUTE accepts the listed DDL and DCL only in the existing allowed contexts, while planFlagIsDDL and the documented isolation behavior remain unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
sql
Domain
databases
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.