cockroachdb / cockroachdb/cockroach
sql/plpgsql: gate dynamic EXECUTE behind a reserved cluster setting
- 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.**
Dynamic SQL in PL/pgSQL (the `EXECUTE` statement) is being added incrementally, starting with a deliberately narrow MVP: no `INTO` or `USING`, stored procedures only, top-level body statements only, no DDL/DCL, and the result of a dynamic `SELECT` discarded. See cockroachlabs/cockroach#2291.
Today the only gate on that work is the `V26_4` cluster version. Once the upgrade to v26.4 finalizes, the feature is unconditionally on for every cluster and there is no way to turn it off.
That is uncomfortable because it is not yet settled whether we will have the bandwidth to finish dynamic SQL. If the MVP ships and investment then stops, we are committed to supporting a visibly partial feature — users will hit the unimplemented forms and file issues against a surface we do not intend to complete in the near term.
We want the code to keep landing on master and running in CI, without exposing the feature to customers before we commit to it.
**Describe the solution you'd like**
Add a boolean cluster setting, defaulting to `false`, registered with the default `settings.Reserved` visibility (i.e. without `settings.WithPublic`) — for example `sql.plpgsql.dynamic_execute.enabled`.
Reserved visibility gives exactly the property we want:
- `SHOW CLUSTER SETTINGS` filters on `public IS TRUE` ([show_all_cluster_settings.go:85](https://github.com/cockroachlabs/cockroach/blob/e6dc8ee414e8daed7251e4ca8429220a241e8d58/pkg/sql/delegate/show_all_cluster_settings.go#L85)), so the setting does not appear there and is excluded from the generated settings documentation.
- It remains settable, and visible via `SHOW ALL CLUSTER SETTINGS` and `crdb_internal.cluster_settings`, so logictests and roachtests can enable it explicitly.
Combine it with the existing version gate behind a single helper, mirroring the established pattern at [clustersettings.go:204](https://github.com/cockroachlabs/cockroach/blob/e6dc8ee414e8daed7251e4ca8429220a241e8d58/pkg/sql/sqlclustersettings/clustersettings.go#L204) and [clustersettings.go:218](https://github.com/cockroachlabs/cockroach/blob/e6dc8ee414e8daed7251e4ca8429220a241e8d58/pkg/sql/sqlclustersettings/clustersettings.go#L218), so that every call site goes through one predicate rather than checking the version and the setting separately. The `*ast.DynamicExecute` case in `buildPLpgSQLStatements` is the single place the version gate is applied today, so this is a small change.
Work items:
- [ ] Register the setting with `Reserved` visibility and a `false` default.
- [ ] Add a `DynamicExecuteEnabled(ctx, st)` helper combining the setting with the `V26_4` gate; replace the existing version check with it.
- [ ] Enable the setting in `plpgsql_execute` and any other logictest file exercising `EXECUTE`.
- [ ] Decide and pin the behavior when the setting is disabled *after* a routine containing `EXECUTE` was created. The optbuilder path runs at both `CREATE` and `CALL`, so the natural outcome is that `CALL` also fails — a reasonable choice for a hidden flag, but it should be an explicit decision with a test rather than an accident.
- [ ] When we commit to the feature, flip to `settings.WithPublic` and a `true` default, and add a release note at that point.
**Describe alternatives you've considered**
- **`settings.WithUnsafe` instead of `Reserved`.** Stronger — updating the setting requires the unsafe-setting interlock. That is more friction than we need for a feature flag we ourselves flip in CI, and it complicates test setup.
- **A session variable instead of a cluster setting.** Wrong granularity: a per-session switch would let one session create a routine that another session cannot call, and it does not give operators a single off switch.
- **Hold the work out of master until dynamic SQL is complete.** Loses incremental review and CI coverage on a large change, and the branch would need continual rebasing.
- **Leave the version gate as the only control.** This is the status quo and provides no off switch once v26.4 finalizes.
**Additional context**
The setting is a project-management control, not a compatibility one — the `V26_4` version gate is still required and still does the rolling-upgrade job of ensuring no node encounters a routine it cannot build. The two gates are independent and both must pass.
Because the setting is `Reserved` and defaults to `false`, no release note is needed when it is introduced; the user-visible change happens when it is made public.
Epic CRDB-48117
Jira issue: CRDB-66343
Contributor guide
Assessment
This issue has not been assessed yet.