cockroachdb / cockroachdb/cockroach
sql/opt: nil pointer dereference projecting virtual computed column after CREATE OR REPLACE FUNCTION flips referenced UDF to VOLATILE
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
When a virtual computed column's expression references a UDF that was created `IMMUTABLE`, and the UDF is later weakened to `VOLATILE` (or `STABLE`) — via `CREATE OR REPLACE FUNCTION` or `ALTER FUNCTION` — CockroachDB accepts the volatility change even though the virtual column requires `IMMUTABLE`. Projecting the virtual column afterwards crashes the optimizer with a nil pointer dereference in `buildProjectionsItemProps`.
The crash is specific to **virtual** computed columns. Unlike stored columns, virtual columns are re-evaluated at read time in the optimizer (`optbuilder.buildScan`). Once the referenced function is no longer `IMMUTABLE`, the computed expression is not built, so `tabMeta.ComputedCols[col]` is nil, producing the nil dereference. Stored columns, `CHECK` constraints, and expression indexes are materialized and do not hit this path.
**To Reproduce**
```sql
CREATE FUNCTION pwn(x INT) RETURNS INT LANGUAGE SQL IMMUTABLE AS $$
SELECT x
$$;
CREATE TABLE t (
pk INT PRIMARY KEY,
x INT,
c INT AS (pwn(x)) VIRTUAL
);
INSERT INTO t (pk, x) VALUES (1, 10);
-- Volatility flipped while the virtual column still references pwn().
CREATE OR REPLACE FUNCTION pwn(x INT) RETURNS INT LANGUAGE SQL VOLATILE AS $$
SELECT x
$$;
SELECT c FROM t;
-- pq: internal error: runtime error: invalid memory address or nil pointer dereference
```
The same crash is reachable via `ALTER FUNCTION pwn(x INT) VOLATILE;` in place of the `CREATE OR REPLACE`, since the read path does not care how the volatility was weakened.
**Expected behavior**
CockroachDB should not crash. The volatility-weakening DDL should be rejected with a clear error while a virtual computed column depends on the function.
This is a CockroachDB-specific fix, not a mirror of PostgreSQL (verified against PG 18.3):
- PG does not allow UDFs in virtual generated columns at all — it rejects the table/column creation up front (`generation expression uses user-defined function` / "Virtual generated columns that make use of user-defined functions are not yet supported"). So PG never reaches this state.
- For the cases PG *does* allow (STORED generated columns, `CHECK` constraints, expression indexes), PG **accepts** weakening the function's volatility via both `CREATE OR REPLACE FUNCTION` and `ALTER FUNCTION`. It enforces `IMMUTABLE` only when the dependent is created, then trusts the user; the stored value or index may become silently stale.
Because CockroachDB *does* allow UDFs in virtual computed columns (which PG forbids), CockroachDB has to guard the volatility change itself for that case, and the error wording is CockroachDB-original.
**Proposed fix (targeted)**
1. Reject the volatility weakening at DDL time when a **virtual** computed column depends on the function. This must cover both `CREATE OR REPLACE FUNCTION` and `ALTER FUNCTION`, in both the legacy and declarative schema changers — ideally through a single shared helper so the paths cannot drift.
2. Only trigger the rejection for genuinely `IMMUTABLE`-required read-time contexts. In particular, do **not** reject functions used only in a column `DEFAULT`/`ON UPDATE` expression — those are permitted to be `VOLATILE` (they are evaluated at write time), so blocking them would be a compatibility regression.
3. Add a defensive check in `optbuilder.buildScan` that turns the nil dereference into a clear internal error, so any descriptor that already reached the corrupt state fails cleanly instead of panicking.
Scope decision left open: whether to also reject volatility weakening for STORED columns, `CHECK` constraints, and expression indexes. Those do not crash and PG accepts them, so guarding them would be an intentional divergence to prevent silent staleness rather than part of the crash fix. Recommend handling separately if desired.
**Additional data**
Stack trace from a recent build:
```
pq: internal error: runtime error: invalid memory address or nil pointer dereference
HINT: You have encountered an unexpected error.
DETAIL: stack trace:
pkg/util/errorutil/catch.go:24: shouldCatch()
pkg/util/errorutil/catch.go:38: MaybeCatchPanic()
GOROOT/src/runtime/panic.go:860: gopanic()
GOROOT/src/runtime/panic.go:336: panicmem()
GOROOT/src/runtime/signal_unix.go:931: sigpanic()
pkg/sql/opt/memo/logical_props_builder.go:1837: buildProjectionsItemProps()
bazel-out/.../bin/pkg/sql/opt/memo/expr.og.go:9079: PopulateProps()
bazel-out/.../bin/pkg/sql/opt/norm/factory.og.go:14165: ConstructProjectionsItem()
pkg/sql/opt/optbuilder/select.go:843: func1()
pkg/sql/opt/colset.go:78: func2()
pkg/util/intsets/fast.go:148: ForEach()
pkg/sql/opt/colset.go:78: ForEach()
pkg/sql/opt/optbuilder/select.go:842: buildScan()
pkg/sql/opt/optbuilder/select.go:162: buildDataSource()
pkg/sql/opt/optbuilder/select.go:92: buildDataSource()
pkg/sql/opt/optbuilder/select.go:1491: buildFromTablesRightDeep()
pkg/sql/opt/optbuilder/select.go:1468: buildFromTables()
pkg/sql/opt/optbuilder/select.go:1393: buildFrom()
pkg/sql/opt/optbuilder/select.go:1299: buildSelectClause()
pkg/sql/opt/optbuilder/select.go:1223: buildSelectStmtWithoutParens()
pkg/sql/opt/optbuilder/select.go:1175: func1()
pkg/sql/opt/optbuilder/with.go:110: processWiths()
pkg/sql/opt/optbuilder/select.go:1174: buildSelect()
pkg/sql/opt/optbuilder/builder.go:418: buildStmt()
pkg/sql/opt/optbuilder/builder.go:340: buildStmtAtRootWithScope()
pkg/sql/opt/optbuilder/builder.go:316: buildStmtAtRoot()
pkg/sql/opt/optbuilder/builder.go:295: Build()
pkg/sql/plan_opt.go:1020: buildExecMemo()
pkg/sql/plan_opt.go:361: makeOptimizerPlanInternal()
pkg/sql/plan_opt.go:341: makeOptimizerPlan()
pkg/sql/conn_executor_exec.go:3509: makeExecPlan()
pkg/sql/conn_executor_exec.go:3070: dispatchToExecutionEngine()
```
Jira issue: CRDB-64604
Contributor guide
Assessment
This issue has not been assessed yet.