cockroachdb / cockroachdb/cockroach
sql/plpgsql: out-of-range placeholder in a lazily built routine body reads the caller's placeholder value
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
An out-of-range ordinal parameter reference in a routine body (e.g. `$1` in a
routine with zero parameters) is normally rejected with
`42P02 no value provided for placeholder`, because the body is built when the
routine is created.
When a body statement is instead built *lazily*, that check does not happen. The
placeholder is resolved against whatever `evalCtx` is active at build time —
which, for a lazily built body, is the enclosing query's. If the caller is a
prepared statement with placeholders, the routine silently reads the *caller's*
placeholder value instead of erroring.
The trigger is a deferred body seam, not any single feature. Two independent
things create one:
- A DDL statement earlier in the body, which defers everything after it. This
requires `sql.procedures.plpgsql.late_binding.enabled = true`.
- Dynamic `EXECUTE`, whose command string is only known at execution time. This
is always deferred and needs no cluster setting.
Observed behavior across the combinations:
| Body | Late binding | Result |
|---|---|---|
| Static SQL | off | `42P02` at `CREATE PROCEDURE` |
| Static SQL, no DDL | on | `42P02` when the body is first built |
| Static SQL, DDL earlier in body | on | **reads caller's `$1`** |
| Dynamic `EXECUTE` | n/a | **reads caller's `$1`** |
Note that late binding alone is not sufficient — it only changes *when* the body
is built, and the reference is still caught as long as something builds it
eagerly.
**To Reproduce**
Static SQL, deferred by a preceding DDL:
```sql
SET CLUSTER SETTING sql.procedures.plpgsql.late_binding.enabled = true;
CREATE PROCEDURE p() LANGUAGE PLpgSQL AS $$
BEGIN
CREATE TABLE d (x INT); -- forces lazy building of the following statement
INSERT INTO d VALUES ($1); -- p has zero parameters, so $1 is out of range
END $$;
CREATE FUNCTION f() RETURNS INT LANGUAGE PLpgSQL AS $$
BEGIN CALL p(); RETURN 0; END $$;
PREPARE q AS SELECT f(), $1::INT;
EXECUTE q(99);
SELECT x FROM d;
```
```
x
-----
99 <- the caller's $1
```
Dynamic `EXECUTE`, with late binding off:
```sql
CREATE TABLE d_exec (x INT);
CREATE PROCEDURE p_exec() LANGUAGE PLpgSQL AS $$
BEGIN
EXECUTE 'INSERT INTO d_exec VALUES ($1)';
END $$;
CREATE FUNCTION f_exec() RETURNS INT LANGUAGE PLpgSQL AS $$
BEGIN CALL p_exec(); RETURN 0; END $$;
PREPARE q_exec AS SELECT f_exec(), $1::INT;
EXECUTE q_exec(77);
SELECT x FROM d_exec;
```
```
x
-----
77 <- the caller's $1
```
Both reproduce on `master`.
**Expected behavior**
An out-of-range `$N` in a routine body should be rejected the same way
regardless of when the body is built — `42P02 no value provided for
placeholder: $N`. A routine body should never resolve a placeholder against the
calling query's parameters.
**Additional data / schema**
Root cause is two fall-throughs in the optbuilder:
- [scope.go:1138](https://github.com/cockroachlabs/cockroach/blob/1f38a33e9a79375d9a9225463fb584b02eb5954d/pkg/sql/opt/optbuilder/scope.go#L1138) —
`case *tree.Placeholder` replaces the placeholder with a routine-argument
column via `findFuncArgCol(t.Idx)`. When the ordinal is out of range that
returns nil and the case falls through, leaving the placeholder in place
rather than raising an error.
- [scalar.go:411](https://github.com/cockroachlabs/cockroach/blob/1f38a33e9a79375d9a9225463fb584b02eb5954d/pkg/sql/opt/optbuilder/scalar.go#L411) —
the surviving placeholder then hits `case *tree.Placeholder`, and because
`b.evalCtx.HasPlaceholders()` is true for a body built during execution of an
outer prepared statement, it is folded to a constant via `eval.Expr` using the
caller's `evalCtx.Placeholders`.
When the body is built eagerly, `evalCtx` has no placeholders, so
`ConstructPlaceholder` is built instead and execution later fails with 42P02 —
hence the divergence.
The `NOTE` already on scope.go:1141 anticipates a related problem (placeholders
belonging to a prepared statement rather than to routine arguments).
**Additional context**
Impact is silent wrong results and unintended writes rather than a crash. The
values read belong to the caller's own prepared statement, so this is not
cross-session data access; the notable case is a routine owned by another user,
whose body can observe parameters the caller passed to the surrounding query.
Likely fix direction: reject an out-of-range ordinal at scope.go:1138 instead of
falling through, so the error is raised wherever the body is built. That path is
shared with SQL-language routines, so it needs checking against
`KeepPlaceholders` and the prepared-statement cases the existing NOTE calls out.
Found during review of the dynamic `EXECUTE` work (cockroachlabs/cockroach#2291,
thread on `plpgsql.go:3575`). Not introduced by that PR — it predates it and
affects any lazily built routine body.
Jira issue: CRDB-66294
Epic CRDB-65937
Contributor guide
Research direction
Start with the placeholder handling at scope.go:1138 and scalar.go:411, then reproduce both cases using the SQL examples in the issue. Check how the proposed rejection interacts with KeepPlaceholders and the prepared-statement cases mentioned in the existing NOTE. Done means out-of-range routine placeholders consistently raise 42P02 without regressing those cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100