On Mon, Apr 06, 2026 at 03:19:48PM -0400, Tom Lane wrote: > I pushed v13 after a tiny bit of additional tweaking.
I had Opus 5 review d516974 "Support more object types within CREATE SCHEMA". It raised a problem in a9c350d "Don't try to re-order the subcommands of CREATE SCHEMA": > +## 4. Findings that belong to the sibling commit a9c350d, not to d516974 > +The common cause: `a9c350d` deleted `setSchemaName()`, which used to write > the new schema's > +name into each element's `RangeVar->schemaname` [...] From then on the new > schema > +reaches its own elements solely through a prepended `search_path` entry > + [...] ISO/IEC 9075-2 5.4 SR 4)b) (found.txt:8701-8702) reads verbatim > +"If the <local or schema qualified name> is contained in a <schema > definition>, then the > +<schema name> that is specified or implicit in the <schema definition> is > implicit." The full review, attached, contains several examples. Some are exotic. Here's one that resonates more with me, adapted slightly from the a9c350d commit message: CREATE TABLE public.mytable (c int); CREATE SCHEMA myschema CREATE VIEW myview AS SELECT * FROM mytable CREATE TABLE mytable (c int); SET search_path = pg_catalog; \d+ myschema.myview RESET search_path; In v18, myview binds to myschema.mytable. In v19, it binds to public.mytable. I think the above-described syntax rule about <local or schema qualified name> doesn't allow v19's interpretation. Stepping back, I am concerned about the complexity v19 is incurring to extend CREATE SCHEMA ... CREATE. The syntax gives only modest benefits, since you can achieve the same object creation with freestanding SQL statements. The above example is indicative of the unusual semantic challenges. CREATE SCHEMA ... CREATE also handicaps the parsing of every command it supports, since parsing can't rely on semicolons to find command boundaries. I grant this project closed one gap in PostgreSQL SQL standard compliance. I'm concerned about painting ourselves into a corner with respect to future syntax needs. Here are my notes on the d516974 findings in the report, only one of which I consider a non-negligible bug in d516974. > +### D1 — psql (and pgbench) do not end a `CREATE SCHEMA` at its semicolon > when a `GRANT` clause follows a routine clause This title overstates the connection to $SUBJECT. As the report goes on to explain, this is a variant of an older bug of psql detecting "begin" in the wrong contexts: -- works CREATE FUNCTION x1f() RETURNS int LANGUAGE sql SET search_path = "begin" AS 'select 1'; select 'ended'; -- psql wrongly concludes command is not yet terminated CREATE FUNCTION x1f() RETURNS int LANGUAGE sql SET search_path = begin AS 'select 1'; select 'ended'; This doesn't affect pg_dump, fortunately, which always quotes the GUC value. > +### D2 — a `SET role` / `SET session_authorization` clause on a routine > element is refused under `AUTHORIZATION`, naming a security-definer function > that does not exist I would not act on this. > +### D3 — PL/pgSQL cannot contain a `CREATE SCHEMA` whose element is a `BEGIN > ATOMIC` routine I think this does qualify as a bug in d516974, but it's not very concerning. > +### D4 — psql offers `COLLATION` and `TYPE` as `CREATE SCHEMA` clauses but > does not complete them I would not act on this.
commit 8a98da0 (HEAD, cqla/create-schema-defect-tests) Author: Noah Misch <[email protected]> AuthorDate: Thu Sep 3 03:43:31 2026 +0000 Commit: Noah Misch <[email protected]> CommitDate: Thu Sep 3 03:43:31 2026 +0000 Audit of d516974 (more object types in CREATE SCHEMA): report and tests Automated, model-driven audit of commit d516974 ("Support more object types within CREATE SCHEMA.") for user-visible defects still present in master (6885b84). Everything here was written by a language model and has had no human review; see PROVENANCE.md. CREATE_SCHEMA_DEFECT_REPORT.md reports four defects attributable to d516974, none severe: D1 psql (and pgbench) do not end a CREATE SCHEMA at its semicolon when a GRANT clause follows a CREATE FUNCTION/PROCEDURE clause: the routine clause's BEGIN/END tracking state leaks into the GRANT, which never resets it because GRANT is not introduced by CREATE. The statement is merged with everything after it, so a following VACUUM fails with "cannot run inside a transaction block" and later statements never run. D2 A SET role / SET session_authorization clause on a routine element is refused under AUTHORIZATION <other role>, with a message naming a security-definer function that does not exist. D3 PL/pgSQL cannot contain a CREATE SCHEMA whose element is a BEGIN ATOMIC routine; pl_gram.y still uses psql's pre-049b742 heuristic. D4 psql offers COLLATION and TYPE as CREATE SCHEMA clauses but then does not complete them. The report also has sections for findings belonging to the sibling commit a9c350d rather than to d516974, for what was examined and found correct, and for the 16 of 19 candidates that were investigated and rejected -- most because the same behavior reproduces outside CREATE SCHEMA entirely, on builds that predate the whole series. There is no fix here, so the added tests encode what a correct implementation would print and therefore FAIL on master by design; the failure diff is the demonstration. Only test files are touched. With the additions, regress (create_schema), plpgsql (plpgsql_misc), psql/001_basic subtests 135-137 and psql/010_tab_completion subtests 110-111 fail, each only on the new material, and three further meson tests fail collaterally because they run the core parallel_schedule. Every added control passes. With the six test files stashed, all six suites are green (243+13+134+101+2+5 subtests, 0 failures). Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01AfvwDcK3tFM4H1JeQXz2Jz --- CREATE_SCHEMA_DEFECT_REPORT.md | 1203 ++++++++++++++++++++++++++ PROVENANCE.md | 305 +++++++ src/bin/psql/t/001_basic.pl | 40 + src/bin/psql/t/010_tab_completion.pl | 95 ++ src/pl/plpgsql/src/expected/plpgsql_misc.out | 35 + src/pl/plpgsql/src/sql/plpgsql_misc.sql | 31 + src/test/regress/expected/create_schema.out | 78 ++ src/test/regress/sql/create_schema.sql | 60 ++ 8 files changed, 1847 insertions(+) diff --git a/CREATE_SCHEMA_DEFECT_REPORT.md b/CREATE_SCHEMA_DEFECT_REPORT.md new file mode 100644 index 0000000..cabc4f3 --- /dev/null +++ b/CREATE_SCHEMA_DEFECT_REPORT.md @@ -0,0 +1,1203 @@ +# Audit of d516974 — "Support more object types within CREATE SCHEMA." + +## Verdict + +Four user-visible defects attributable to commit `d516974840f4059d331ae6057ede3e4edd3c6747` +survive in master (`6885b845b4ba0b7aee09daa9817703477faa3704`), and none of them is severe. +Two are in psql — its statement lexer merges a `CREATE SCHEMA` with everything after it when +a `GRANT` clause follows a routine clause (D1), and its tab completion offers `COLLATION` and +`TYPE` as `CREATE SCHEMA` clauses but then does not complete them (D4). One is in PL/pgSQL's +own statement lexer, which cannot express a `CREATE SCHEMA` whose element is a `BEGIN ATOMIC` +routine (D3). One is server-side: a `SET role` / `SET session_authorization` clause on a +routine element is refused under `AUTHORIZATION` with a message naming a security-definer +function that does not exist (D2). +The server-side heart of the commit is in good shape: the newly-allowed sub-commands +produce catalog contents, dependencies, ownership, event-trigger payloads, error positions +and `pg_dump` output that match the same commands issued separately under an equivalent +`search_path` — byte for byte, apart from pg_dump's per-dump `\restrict` nonce — and 16 hunt +lenses plus a refute stage found no crash, no assertion failure, no privilege escalation and +no data corruption *attributable* to the new syntax. (Two hazards of that kind are reachable +*through* it and are reported below because they matter, but neither is caused by it: the +confused deputy of §4 S2, and the dangling-`pronamespace` catalog corruption of §6 C06 — +each reproduces on `pre` with no `CREATE SCHEMA` element list involved at all.) +The most serious problems this audit found in the CREATE SCHEMA area — the `"$user"` schema +that receives none of its own elements, the session temp schema outranking the new schema, +and an element's user code hijacking the temporary `search_path` — all belong to the sibling +commit **a9c350d**, not to d516974; they are reported in §4 as context. Sixteen of the nineteen +hunt candidates were rejected outright, together with orchestrator findings O2 and O4 (§6) — +most because the same behaviour reproduces outside `CREATE SCHEMA` entirely, on builds that +predate the whole series. + +--- + +## 1. Scope + +**Commit audited:** `d516974840f4059d331ae6057ede3e4edd3c6747`, Tom Lane, 2026-04-06, +"Support more object types within CREATE SCHEMA." It adds `CreateDomainStmt`, +`CreateFunctionStmt` and `DefineStmt` to the `schema_stmt` nonterminal +(`src/backend/parser/gram.y:1686-1696`), adds `checkSchemaNameList()` and the new node cases +in `transformCreateSchemaStmtElements()` (`src/backend/parser/parse_utilcmd.c`), tweaks +psql tab completion and the psql lexer, and extends `create_schema.sgml`. Because the +grammar's `DefineStmt` also produces `CompositeTypeStmt`, `CreateEnumStmt` and +`CreateRangeStmt`, the newly-legal clauses are: `CREATE [OR REPLACE] FUNCTION`/`PROCEDURE`, +`CREATE DOMAIN`, `CREATE [OR REPLACE] AGGREGATE` (modern and pre-8.2 syntax), +`CREATE OPERATOR`, `CREATE TYPE` (shell, base, `AS (...)`, `AS ENUM`, `AS RANGE`), +`CREATE COLLATION` (definition / `FROM` / `IF NOT EXISTS`) and +`CREATE TEXT SEARCH PARSER | DICTIONARY | TEMPLATE | CONFIGURATION`. + +**Base commit:** `404db8f9edbb` (= `d516974^`). + +**Sibling commits, in the same series, NOT audited here:** +`a9c350d9ee66` "Don't try to re-order the subcommands of CREATE SCHEMA." (= the commit that +replaced per-element schema-name rewriting with a temporarily prepended `search_path`), and +`404db8f9edbb` "Execute foreign key constraints in CREATE SCHEMA at the end." +`a9c350d^` is `1ff3180ca016`. One later follow-up already in master: +`049b742daad0` (2026-06-23) "psql: Tighten heuristics for BEGIN/END within CREATE SCHEMA." + +### What "defect" means in this report + +Every candidate was scored on three independent booleans, and a candidate is a defect iff +`per_principles OR (sql_mismatch AND is_new)`: + +* **`sql_mismatch`** — the SQL standard requires behaviour master does not exhibit. Text + consulted: ISO/IEC 9075-2:200x committee draft at `/home/nm/src/pg/gbyrun/std/found.txt`. + No standard requirement is asserted in this report without a verbatim quotation. +* **`per_principles`** — the commit's *own* principles require behaviour master does not + exhibit: its commit message, the code comments it wrote, the documentation it wrote, and + the plain proposition that a command `create_schema.sgml` lists as accepted inside + `CREATE SCHEMA` should behave inside `CREATE SCHEMA` the way it behaves outside, apart + from landing in the new schema. Crashes, assertion failures, wrong catalog contents, + silently-wrong results and misleading error messages are always violations. +* **`is_new`** — the *rule violation* (not merely the syntax) is new relative to `d516974^`. + New syntax alone is not a defect. + +Attribution keys on the rule violation, not on which file the commit touched. A finding +that reproduces on `old` with a pre-d516974-legal element, or outside `CREATE SCHEMA` +entirely, is not a d516974 finding; those are in §4 and §6. + +### Reference builds + +| name | commit | meaning | server version | +|---|---|---|---| +| `new` | `6885b845b4ba` | master, **with** d516974 | 20devel | +| `old` | `404db8f9edbb` | `d516974^`, with a9c350d and 404db8f | 19devel | +| `pre` | `1ff3180ca016` | `a9c350d^`, before the whole series | 19devel | + +All three are meson builds with `cassert` and debug. `a9c350d` is an ancestor of `404db8f`; +`d516974` is an ancestor of master; `git rev-list --count 404db8f..6885b84` = **1271**, so no +`is_new` claim below rests on the new-vs-old behavioural delta alone — each is anchored in +`git blame`/source or in a probe using pre-d516974 syntax. + +--- + +## 2. Findings attributable to d516974 + +Severity order. None is a crash, an assertion failure, a privilege escalation, or a source +of catalog corruption; the cassert server logs are clean (`PANIC|FATAL|TRAP|Assert`) on every +run cited in this report. + +--- + +### D1 — psql (and pgbench) do not end a `CREATE SCHEMA` at its semicolon when a `GRANT` clause follows a routine clause + +**Severity: low-to-medium. New in d516974 (narrowed, not fixed, by 049b742).** + +**One-line statement.** A `CREATE [OR REPLACE] FUNCTION`/`PROCEDURE` clause arms +`psqlscan.l`'s BEGIN/END counter; the per-clause reset that is supposed to disarm it fires +only on the token `create`, and `GrantStmt` is the one `schema_stmt` alternative that does +not begin with `CREATE` — so an unquoted identifier `begin` inside a following `GRANT` +clause is counted as the start of a routine body, and psql sends the `CREATE SCHEMA` and +everything after it as a single query. + +**Minimal reproduction** (build `new`; no role setup needed — the poisoning identifier is a +view the same statement creates): + +```sql +CREATE SCHEMA cs_gb + CREATE VIEW begin AS SELECT 1 AS one + CREATE FUNCTION cs_gb_f() RETURNS int LANGUAGE sql AS 'select 1' + GRANT SELECT ON begin TO public; +VACUUM; +SELECT 'split ok' AS result; +``` + +Run as `psql -X -q -A -t -v ON_ERROR_STOP=1 -f defect.sql`. + +**Literal observed output** (`/home/nm/src/pg/csxrun/evidence/CLIENT-C03-oracle-new.txt`): + +``` +############ CASE: defect ############ +psql:/tmp/tmp.vkud8hwplT/defect.sql:6: ERROR: VACUUM cannot run inside a transaction block +psql exit=3 +-- schemas that exist now -- + +``` + +The blank line is the point: `cs_gb` does not exist. The script contains no transaction +block. The server confirms the merge — with `log_statement=all` a *single* +`LOG: statement:` entry carries all of the input +(`/home/nm/src/pg/csxrun/evidence/ADJ-C03-A-new.txt`, `refute-C03-run2-allbuilds.txt`). + +**What a correct implementation produces, and why.** Changing nothing but the quoting of +the same view name — `GRANT SELECT ON "begin" TO public` — is semantically identical SQL +(`begin` is an unreserved keyword) and lexes correctly. Same file, same run: + +``` +############ CASE: oracle ############ +split ok +psql exit=0 +-- schemas that exist now -- +cs_gb_oracle +``` + +so the oracle is master's own behaviour one quoting change away. The control — same shape +with `CREATE VIEW` in place of the routine clause — also splits correctly (`CASE: control` +in the same transcript, `cs_gc` created). + +**The worst symptom is in an interactive terminal, where there is no diagnostic at all.** +Real pty, `--no-psqlrc`, `INPUTRC=/dev/null` +(`/home/nm/src/pg/csxrun/evidence/CRIT-3-interactive-new.txt`, build `new`): + +``` +----- typed: +CREATE SCHEMA cs1 + CREATE FUNCTION f() RETURNS int LANGUAGE sql AS 'select 1' + GRANT ALL ON SCHEMA cs1 TO begin; +SELECT 'MARKER_P' AS m; +\echo ZZ_PROBE_END +----- psql showed: +CREATE SCHEMA cs1 +postgres-# CREATE FUNCTION f() RETURNS int LANGUAGE sql AS 'select 1' +postgres-# GRANT ALL ON SCHEMA cs1 TO begin; +postgres-# SELECT 'MARKER_P' AS m; +postgres-# \echo ZZ_PROBE_END +ZZ_PROBE_END +postgres-# +``` + +The user typed a complete, semicolon-terminated statement; psql printed nothing, sent +nothing to the server, and silently sat on the continuation prompt. Every later statement +is swallowed the same way. The only exits are `\r` (which discards everything typed since +the wedge) or `\g`. On `old` the identical keystrokes return the prompt to `postgres=#` and +run the next statement (`CRIT-3-interactive-old.txt`). This consequence was found by the +completeness critic and did **not** pass through the refute stage; it was not run on `pre`. + +**Mechanism** (against 6885b84). + +* `src/fe_utils/psqlscan.l:1102-1107` — the per-clause reset: + `if (pg_strcasecmp(identifier, "create") == 0) { state->sub_idents_count = 0; ... }`. +* `src/backend/parser/gram.y:1686-1696` — `GrantStmt` is the only `schema_stmt` alternative + that does not begin with `CREATE`, so the reset never fires for a `GRANT` clause and + `sub_idents` still spells `c`,`f` from the preceding routine clause. +* `src/fe_utils/psqlscan.l:1118-1123` — with `psqlscan_is_create_routine(state->sub_idents)` + still true, the identifier `begin` increments `begin_depth`. +* `src/fe_utils/psqlscan.l:1099` — the whole per-clause block is skipped once + `begin_depth > 0`, so no later clause can recover. An unquoted `end` would decrement, but + `END` is reserved in a `GRANT` clause and a double-quoted `"end"` is not tracked at all. +* The `;` rule then does not terminate the statement, and psql keeps accumulating. + +**Rubric scores.** + +* `sql_mismatch = false`. 11.1 `<schema definition>` (found.txt:25906-25953) contains no + statement terminator, and the only place the standard terminates a *directly invoked* + statement with a semicolon is 22.1 (`<direct SQL statement> ::= <directly executable + statement> <semicolon>`, found.txt:50046-50047) — which 4.25 immediately disclaims: + "In direct invocation of SQL, the method of invoking <direct SQL statement>s ... are + implementation-defined" (found.txt:4840-4844). A client's script-splitting heuristic is + exactly that implementation-defined method. No standard citation is offered for this + finding. +* `per_principles = true`. Master's own comment at `src/fe_utils/psqlscan.l:1093-1095` + states the property that is violated: "In CREATE SCHEMA, track identifiers from each + top-level CREATE schema element separately, so that BEGIN/END tracking is enabled only + within CREATE [OR REPLACE] {FUNCTION|PROCEDURE} clauses." It is not. `049b742`'s commit + message repeats the promise — "only counting BEGIN/END within those clauses". (Both are + 049b742's text, not d516974's; d516974's own hedge — "a bit shaky but should be okay with + the present set of valid subcommands" — no longer exists in master.) d516974's own + documentation lists `CREATE FUNCTION`, `CREATE PROCEDURE` and `GRANT` together as accepted + clauses (`doc/src/sgml/ref/create_schema.sgml:104-120`) under a Description paragraph + promising that "The subcommands are treated essentially the same as separate commands + issued after creating the schema" (`create_schema.sgml:61-65`; that paragraph is + 3450fd08/2003, so it is corroboration of master's standing principle rather than + documentation this commit wrote). Consequences are two of the rubric's automatic + triggers: a silently-wrong result (psql prints `CREATE SCHEMA` for a schema that is then + rolled back) and a misleading error (`VACUUM cannot run inside a transaction block` for a + script containing no transaction block). +* `is_new = true`. `csx-old/src/fe_utils/psqlscan.l` and `csx-pre/src/fe_utils/psqlscan.l` + are byte-identical (`diff -q`), and neither records the token `schema` at all, so + `begin_depth` could never rise inside a `CREATE SCHEMA` before d516974. Behaviourally, + on `old` the same input is terminated at its semicolon + (`/home/nm/src/pg/csxrun/evidence/ADJ-C03-A-old.txt` CASE 1: the server reports + `ERROR: syntax error at or near "FUNCTION"` for the `CREATE SCHEMA` alone and the + following statement runs). The arming prefix — `CREATE [OR REPLACE] {FUNCTION|PROCEDURE}` + as a schema clause — is exactly what d516974 legalized. + +**This is NOT the case 049b742 deferred** — the discriminator matters, because a reviewer +will otherwise reject it as already-conceded. 049b742's message defers +`CREATE FUNCTION begin () ...`, "true all along with no field complaints". Measured on +master (`/home/nm/src/pg/csxrun/evidence/XPREM-p6-new.txt`): + +| case | shape | result | +|---|---|---| +| K1 | `CREATE SCHEMA k1 CREATE FUNCTION begin () ...` | merged; `k1 schema exists: 0` | +| K1b | the same routine **at top level** | merged too — the deferred family fails identically outside | +| K2 | this finding: unremarkable `CREATE FUNCTION k2f ()`, `begin` in the following `GRANT` | merged; `k2 schema exists: 0` | +| K2b | the identical routine **and** the identical `GRANT` at top level | **works**: `CREATE FUNCTION` / `GRANT` / `VACUUM` / `K2B_MARKER` | +| K3 | pre-v19-legal `CREATE TABLE` clause + `GRANT ... TO begin` | **works**; `k3 schema exists: 1` | + +K1/K1b are the conceded family; the top-level heuristic is fooled and CREATE SCHEMA +inherits it. K2/K2b are not: the top-level heuristic is *not* fooled by these tokens, and +the leak is across the clause boundary — precisely what 049b742 promised to prevent. K3 +confirms 049b742's other claim, that nothing that worked before v19 was put at risk. + +**Honest narrowing.** The damage *class* is neither new nor CREATE SCHEMA-specific: a plain +top-level `CREATE FUNCTION x1f() RETURNS int LANGUAGE sql SET search_path = begin AS 'select 1';` +swallows a following `VACUUM` identically on master and on `old` +(`XPREM-p3-new.txt`, `XPREM-p3-old.txt`). What is new is the leak across a clause boundary. +The trigger also needs an object named with the unreserved keyword `begin` referenced +unquoted in a `GRANT` clause after a routine clause. + +**Attribution: d516974** (introduced), narrowed by 049b742. + +**Other consequences verified on master, from the same merge:** pgbench (which links the same +`psql_scan()`) reports one command instead of three in simple mode +(`refute-C03-run3.txt` X4); `\i` runs the included file *before* the textually preceding +`CREATE SCHEMA` (`refute-C03-run3.txt` X3, `H13-psqlmodes-new-1.txt` P5); the poisoning +identifier can arrive via `:var` interpolation (`H13-psqlmodes-new-1.txt` P8), so a script +whose text never contains the word `begin` can be poisoned from outside it — with +`psql -X -v r=begin -f`, a file containing only `GRANT USAGE ON SCHEMA v1 TO :r;` in the +`GRANT` clause merges and the schema is rolled back, while `-v r=public` splits correctly +(`FACTCHECK-D1-var-new.txt`). + +**Test on this branch: yes.** `src/bin/psql/t/001_basic.pl`, subtests 135-137 (of 140), +added at lines 580-618 with a passing control at subtests 138-140. Verbatim failure +(`/home/nm/src/pg/csxrun/evidence/FINAL-tap.txt`): + +``` +not ok 135 - CREATE SCHEMA ends at its semicolon: exit code 0 +# got: '3' +not ok 136 - CREATE SCHEMA ends at its semicolon: no stderr +# got: 'psql:<stdin>:6: ERROR: VACUUM cannot run inside a transaction block' +not ok 137 - CREATE SCHEMA ends at its semicolon: matches +# '' +``` + +The expected values encode correct behaviour, so the test fails on master by design. + +--- + +### D2 — a `SET role` / `SET session_authorization` clause on a routine element is refused under `AUTHORIZATION`, naming a security-definer function that does not exist + +**Severity: low. Reachability is new in d516974; the rule violation is reachable on `pre` by +a harder route (see Attribution).** + +**One-line statement.** Inside `CREATE SCHEMA ... AUTHORIZATION <role other than the current +user>`, every `SET`/`RESET` clause on a `CREATE [OR REPLACE] FUNCTION`/`PROCEDURE` element +that names `role` or `session_authorization` is rejected — including when the invoker is a +superuser and the `AUTHORIZATION` role is a superuser — with a message that is false. + +**Minimal reproduction** (build `new`, as the bootstrap superuser): + +```sql +CREATE ROLE regress_plan_super SUPERUSER; +CREATE SCHEMA sa1 AUTHORIZATION regress_plan_super + CREATE FUNCTION f1() RETURNS int LANGUAGE sql SET role = 'regress_plan_super' AS 'select 1'; +``` + +**Literal observed output** (`/home/nm/src/pg/csxrun/evidence/PLAN-o1o2-new.txt`): + +``` +CREATE SCHEMA sa1 AUTHORIZATION regress_plan_super + CREATE FUNCTION f1() RETURNS int LANGUAGE sql SET role = 'regress_plan_super' AS 'select 1'; +psql:/home/nm/src/pg/csxrun/probe/plan/o1o2.sql:6: ERROR: cannot set parameter "role" within security-definer function +``` + +Controls in the same transcript: the identical `CREATE FUNCTION` **at top level** succeeds +(`O1d`: `CREATE FUNCTION`); the identical element inside `CREATE SCHEMA` **without** +`AUTHORIZATION` succeeds (`O1c`: `CREATE SCHEMA`); `AUTHORIZATION CURRENT_USER`, where +`saved_uid == owner_uid` so `SetUserIdAndSecContext` is skipped, succeeds (`O1f`); a plain +non-role `SET` clause under `AUTHORIZATION` succeeds (`O1g`). Every spelling that names +either GUC fails — twelve were enumerated and all twelve error +(`/home/nm/src/pg/csxrun/evidence/FACTCHECK-D2-spellings-new.txt`): `SET role =`, +`SET role TO`, `SET role TO DEFAULT`, `SET role FROM CURRENT`, `RESET role`, the four +`session_authorization` analogues, and `SET SESSION AUTHORIZATION <literal>`, +`SET SESSION AUTHORIZATION DEFAULT`, `RESET SESSION AUTHORIZATION`. `RESET ALL` alone slips +through (`/home/nm/src/pg/csxrun/evidence/h02-a-new.txt` A1, verbatim): + +``` +CREATE SCHEMA a1c AUTHORIZATION regress_bob CREATE FUNCTION f() RETURNS int LANGUAGE sql AS 'select 1' RESET role; +psql:/home/nm/src/pg/csxrun/probe/h02-a.sql:15: ERROR: cannot set parameter "role" within security-definer function +CREATE SCHEMA a1d AUTHORIZATION regress_bob CREATE FUNCTION f() RETURNS int LANGUAGE sql AS 'select 1' RESET session_authorization; +psql:/home/nm/src/pg/csxrun/probe/h02-a.sql:16: ERROR: cannot set parameter "session_authorization" within security-definer function +CREATE SCHEMA a1e AUTHORIZATION regress_bob CREATE FUNCTION f() RETURNS int LANGUAGE sql AS 'select 1' RESET ALL; +CREATE SCHEMA +``` + +Note `RESET role`: nothing is being *set* at all, yet the message says "cannot set +parameter". + +**What a correct implementation produces, and why.** The documented equivalent — +`SET ROLE r; CREATE FUNCTION r_schema.f() ... SET role = 'r';` — succeeds and stores +`proconfig = {role=r}`, and so does the same `CREATE SCHEMA` when `AUTHORIZATION` names the +current role. Verbatim oracle from master +(`/home/nm/src/pg/csxrun/evidence/PLAN-oracle-new.txt`, `AUTHORIZATION CURRENT_ROLE` under +`SET ROLE`): + +``` + proname | nspname | owner | proconfig +------------+------------------------+----------------------------+---------------------------------------------------- + cs_setrole | regress_schema_setrole | regress_create_schema_role | {role=regress_create_schema_role} + cs_setsess | regress_schema_setrole | regress_create_schema_role | {session_authorization=regress_create_schema_role} +(2 rows) +``` + +**Mechanism** (against 6885b84). + +* `src/backend/commands/schemacmds.c:149-150` — + `SetUserIdAndSecContext(owner_uid, save_sec_context | SECURITY_LOCAL_USERID_CHANGE)` for + the whole element body, so `InLocalUserIdChange()` is true (and + `InSecurityRestrictedOperation()` is not). +* `src/backend/utils/misc/guc.c:3541-3553` — the `GUC_NOT_WHILE_SEC_REST` block tests + `InLocalUserIdChange()` *before* consulting `changeVal`, so a validation-only call is + rejected. The comment at `guc.c:3546-3548` itself calls the phrasing "historical, but + it's the most common case". +* Two call paths reach it and a fix must handle both — established by a + `backtrace_functions` trace during the verify pass: first + `CreateFunction -> GUCArrayAdd -> validate_option_array_item -> set_config_option(..., changeVal = false, ...)`, + which is what fails today; then `ProcedureCreate -> ProcessGUCArray(..., GUC_ACTION_SAVE)` + with `changeVal = true`, which applies proconfig around the language validator (gated on + `check_function_bodies`, `pg_proc.c:742-755`). The verify pass reports that gating the + guard on `changeVal` alone still fails, because the second site then hits it; what is + re-verified here is that `check_function_bodies = off` does **not** avoid the error, + because the first site still fires + (`/home/nm/src/pg/csxrun/evidence/VERIFY-o1-mechanism-new.txt`). A fix must also stop + short of actually applying the value: a throwaway patch that + simply dropped the guard PANICked the server on + `Assert(SecurityRestrictionContext == 0)` — `miscinit.c:491`, in `SetOuterUserId()`, + which the `role` GUC's assign path reaches via `SetCurrentRoleId()` (`miscinit.c:981`); + `SetSessionUserId()` carries the same assertion at `miscinit.c:525`. (That throwaway + tree was deleted after the verify pass, so this one item is reported, not re-run, by the + fact-check; the two assertions and both call paths are verified in master's source.) +* Only `role` and `session_authorization` carry `GUC_NOT_WHILE_SEC_REST`; + `functioncmds.c:683-684` short-circuits `VAR_RESET_ALL`, which is why `RESET ALL` escapes. + +**Rubric scores.** + +* `sql_mismatch = false`. `proconfig` SET clauses have no counterpart in ISO/IEC 9075, and + 11.1's Access Rules make the privilege environment implementation-defined. No citation is + offered. +* `per_principles = true`, on two independent grounds. (a) The rubric's automatic trigger: + the message names a security-definer function, and there is none anywhere — at top level a + bad value instead gives the informative `NOTICE: role "..." does not exist` + + `ERROR: role "..." does not exist` (`h02-e-new.txt` E5), so the false message pre-empts the + accurate one. (b) d516974 added `CREATE FUNCTION` and `CREATE PROCEDURE` to the + accepted-clause list it wrote at `create_schema.sgml:104-120`, sitting under a Description + paragraph whose only stated exception is ownership. +* `is_new`: **true for this spelling, false for the rule.** On `old` and `pre` the statement + is `ERROR: syntax error at or near "FUNCTION"`. But on `pre`, using only pre-d516974-legal + elements, the identical false message already appears in the identical environment + (`/home/nm/src/pg/csxrun/evidence/adjC12-a3-pre.txt`, verbatim): + + ``` + CREATE SCHEMA sq AUTHORIZATION regress_bob + CREATE TABLE parent (a int) PARTITION BY RANGE (a) + CREATE TABLE child PARTITION OF parent FOR VALUES FROM (public.fx(1)) TO (public.fx(10)); + psql:/home/nm/src/pg/csxrun/probe/adjC12/a3.sql:9: ERROR: cannot set parameter "role" within security-definer function + ``` + + where `public.fx` merely carries `proconfig = {role=regress_bob}`; the documented + separate-command form in the same transcript succeeds. Because `per_principles` is true, + the verdict does not depend on `is_new`. + +**Attribution: shared.** d516974 owns the reachability (one statement, no setup, from a +clause it added and documented) and the specific *validation-only* instance; the guc.c and +schemacmds.c code on the failing path is untouched by d516974 — +`git show --stat d516974` modifies no file under `src/backend/commands/` or +`src/backend/utils/`. A reviewer who keys attribution strictly on the rule violation should +read this section together with §6's rejected candidate C12 and file it as pre-existing, +newly exposed. Either way the fix belongs in `guc.c` (consult `changeVal`, fix the wording) +or `schemacmds.c` (push a GUC nest level so the userid change need not be "local"), never in +anything d516974 wrote. + +**Also reachable from a `CREATE EXTENSION` script** — superuser-installed and non-superuser +trusted alike (`h02-f-new.txt` F2/F3/F7), where it blocks the natural way to write an +extension that ships a routine with a `SET role` clause into an `AUTHORIZATION`-owned schema. + +**Test on this branch: yes.** `src/test/regress/{sql,expected}/create_schema.{sql,out}`, +block at `sql/create_schema.sql:188-211`; failure hunk `@@ -347,17 +347,17 @@` of +`testrun/regress/regress/regression.diffs` +(`/home/nm/src/pg/csxrun/evidence/FINAL-regressdiffs.txt`): + +``` ++ERROR: cannot set parameter "role" within security-definer function + SELECT p.proname, pg_get_userbyid(p.proowner) AS owner, p.proconfig + FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = 'regress_schema_setrole' ORDER BY 1; +- proname | owner | proconfig +-------------+----------------------------+---------------------------------------------------- +- cs_setrole | regress_create_schema_role | {role=regress_create_schema_role} +- cs_setsess | regress_create_schema_role | {session_authorization=regress_create_schema_role} +-(2 rows) ++ proname | owner | proconfig ++---------+-------+----------- ++(0 rows) + + SET client_min_messages = warning; + DROP SCHEMA regress_schema_setrole CASCADE; ++ERROR: schema "regress_schema_setrole" does not exist +``` + +--- + +### D3 — PL/pgSQL cannot contain a `CREATE SCHEMA` whose element is a `BEGIN ATOMIC` routine + +**Severity: low. New in d516974.** + +**One-line statement.** `pl_gram.y`'s copy of psql's statement-boundary heuristic recognizes +`BEGIN`/`END` only after a leading `CREATE [OR REPLACE] {FUNCTION|PROCEDURE}`, never after +`CREATE SCHEMA`, so it cuts the statement at the first semicolon inside the routine body — +making a construct that `create_schema.sgml` lists as accepted unwritable in PL/pgSQL. + +**Minimal reproduction** (build `new`): + +```sql +DO $$ BEGIN CREATE SCHEMA a_s CREATE FUNCTION a_f() RETURNS int BEGIN ATOMIC SELECT 1; END; END $$; +``` + +**Literal observed output** (`/home/nm/src/pg/csxrun/evidence/adjC14-new-1.txt`, case A): + +``` +DO $$ BEGIN CREATE SCHEMA a_s CREATE FUNCTION a_f() RETURNS int BEGIN ATOMIC SELECT 1; END; END $$; +psql:/home/nm/src/pg/csxrun/probe/adjC14/adj.sql:2: ERROR: syntax error at end of input +LINE 1: ...E FUNCTION a_f() RETURNS int BEGIN ATOMIC SELECT 1; END; END... + ^ +``` + +Controls in the same transcript: the same routine in PL/pgSQL but **not** inside +`CREATE SCHEMA` returns `DO` (case B); the same `CREATE SCHEMA` submitted directly returns +`CREATE SCHEMA` (case C); the same statement via plpgsql `EXECUTE` returns `DO` (case D); an +`EXCEPTION` handler does not catch it, because it is a compile-time failure (case G); and a +named plpgsql function containing it fails at `CREATE FUNCTION` time (case H). The trigger +is exactly one or more statements in the `ATOMIC` body — an empty body works (case F). + +**What a correct implementation produces, and why.** The byte-identical `CREATE SCHEMA` +submitted at top level on master succeeds +(`/home/nm/src/pg/csxrun/evidence/PLAN-oracle4-new.txt`): + +``` +create schema misc_schema + create function test3() returns int + begin atomic + select 3 + 3; + end; +CREATE SCHEMA +CREATE OR REPLACE FUNCTION misc_schema.test3() + RETURNS integer + LANGUAGE sql +BEGIN ATOMIC + SELECT (3 + 3); +END +``` + +so the correct output is a real oracle, not a guess. + +**Mechanism** (against 6885b84). `src/pl/plpgsql/src/pl_gram.y:3032` `make_execsql_stmt()` +keeps `char tokens[4]` (`:3049`) and updates it only while +`tokens[0] == 'c' && token_count < sizeof(tokens)` (`:3104-3105`). For +`CREATE SCHEMA s CREATE FUNCTION ...` the trace is: `CREATE` → `tokens[0]='c'`; +`SCHEMA` → nothing (`schema` is a PL/pgSQL *unreserved keyword*, +`src/pl/plpgsql/src/pl_unreserved_kwlist.h:101`, so `plpgsql_yylex` returns `K_SCHEMA` and +none of the `tok == T_WORD && strcmp(...)` tests at `:3107-3116` can fire); then the name; +then the second `CREATE`; then the block is skipped forever. `in_routine_definition` +(`:3047`) stays false, `begin_depth` stays 0, and the `;` after `SELECT 1` breaks the scan at +`:3136-3137`. The comment at `:3065-3066` still says "We follow psql's lead in not +recognizing BEGIN/END except after CREATE [OR REPLACE] {FUNCTION|PROCEDURE}" — a statement of +intent that this very series made false, since psql's lead moved in d516974 + 049b742 and +`pl_gram.y` was not updated by any commit in the series. + +Structurally this can never be *silent*: `tokens[1]` is the token after `CREATE`, which for +`CREATE SCHEMA` is always `SCHEMA`, so `in_routine_definition` can never be spuriously true +and truncation always cuts a routine body before its `END` (verified with a leading +`CREATE TABLE` clause that would leave a valid prefix — `refC14-r2-new.txt` R2-C still +errors, nothing created). + +**Rubric scores.** + +* `sql_mismatch = false`, deliberately. `grep -ci 'plpgsql\|PL/pgSQL' found.txt` = 0; + 11.1's Syntax Rules constrain what contained elements may name, and nothing in the standard + speaks to how a vendor's procedural language finds the end of an embedded statement. The + server accepts the SQL — cases C and D prove it. +* `per_principles = true`. d516974's own documentation lists `CREATE FUNCTION` and + `CREATE PROCEDURE` as accepted clauses (`create_schema.sgml:104-120`), and + `doc/src/sgml/plpgsql.sgml` still says without qualification that "In general, any SQL + command that does not return rows can be executed within a PL/pgSQL function just by + writing the command." Case B proves the identical routine written as a separate command in + the same `DO` block works, so the documented equivalence fails only for the + in-`CREATE SCHEMA` form. (The "misleading error message" argument is weaker than it looks + and is *not* relied on: in the realistic multi-line form the display is + `LINE 7: select 3 + 3;` with the caret at end of line and nothing after it — + `refC14-r1-new.txt` R1-F, `XPREM-p5-new.txt` — and running the truncated fragment alone + produces the identical message legitimately, `refC14-r2-new.txt` R2-A.) +* `is_new = true`, by reachability rather than by file ownership. On `old` and `pre` the + same `DO` block fails with `ERROR: syntax error at or near "FUNCTION"` + (`adjC14-old-1.txt`, `adjC14-pre-1.txt`, byte-identical) — the backend grammar rejects the + element outright, so no documented-legal element was being refused and there was no rule to + violate. Every pre-d516974-legal element shape with a depth-0 hazard passes on `old`: + semicolon inside a string literal, `CASE ... END` in a view, four chained elements + including `GRANT`, and 049b742's `CREATE VIEW begin` case — all return `DO`, `q1_ok..q4_ok = 1` + (`/home/nm/src/pg/csxrun/evidence/XPREM-p2-old.txt`). + +**Attribution: d516974.** The heuristic in `pl_gram.y` is `57b440ec` (2024) and was not +touched by the series; what d516974 supplies is the documented-legal construct that the +heuristic cannot parse. Note for a fixer: PL/pgSQL is *immune* to the hazard 049b742 fixed +(`DO $$ BEGIN CREATE SCHEMA m5 CREATE VIEW begin AS SELECT 1; END $$;` returns `DO`, +`refC14-r4-new.txt` R4-5) precisely because `in_routine_definition` is never set — so a naive +"count BEGIN/END throughout CREATE SCHEMA" port would import that regression. `pl_gram.y`'s +other known semicolon hazard, `CREATE RULE` with multiple actions, is already covered by its +`paren_depth` counter and works in a `DO` block on all three builds +(`refC14-r1-new.txt` R1-D, `refC14-r3-oldpre.txt` R3-B), so this is the sole outstanding gap +in that heuristic. + +**Test on this branch: yes.** `src/pl/plpgsql/src/{sql,expected}/plpgsql_misc.{sql,out}`, +block at `sql/plpgsql_misc.sql:24-53`, sitting beside the `57b440e` test it mirrors. Single +failure hunk `@@ -55,15 +55,13 @@` (`/home/nm/src/pg/csxrun/evidence/FINAL-regressdiffs.txt`): + +``` ++ERROR: syntax error at end of input ++LINE 7: select 3 + 3; ++ ^ + \sf misc_schema.test3 +-CREATE OR REPLACE FUNCTION misc_schema.test3() +- RETURNS integer +- LANGUAGE sql +-BEGIN ATOMIC +- SELECT (3 + 3); +-END ++ERROR: function "misc_schema.test3" does not exist + drop schema misc_schema cascade; +-NOTICE: drop cascades to function misc_schema.test3() ++ERROR: schema "misc_schema" does not exist +``` + +--- + +### D4 — psql offers `COLLATION` and `TYPE` as `CREATE SCHEMA` clauses but does not complete them + +**Severity: cosmetic. This is the weakest item in this report; the counterarguments are +stated in full below.** + +**One-line statement.** d516974 added `COLLATION` and `TYPE` to the clause keywords psql +offers after `CREATE SCHEMA ... CREATE`, and in the same diff converted two other clause +families (`CREATE DOMAIN`, `CREATE TEXT SEARCH`) from anchored `Matches()` to `TailMatches()` +"so use TailMatches" — but left `CREATE COLLATION`'s and `CREATE TYPE`'s own rules anchored, +so those two complete at top level and not inside `CREATE SCHEMA`. + +**Minimal reproduction** — interactive psql with readline; the audit used +`src/bin/psql/t/010_tab_completion.pl`'s own pty harness. Type +`CREATE SCHEMA s CREATE COLLATION c FR<TAB>` and `CREATE SCHEMA s CREATE TYPE t AS EN<TAB>`. + +**Literal observed output** (`/home/nm/src/pg/csxrun/evidence/FINAL-tap.txt`): + +``` +not ok 110 - complete CREATE COLLATION inside CREATE SCHEMA +# Actual output was "CREATE SCHEMA s CREATE COLLATION c FR\r\rpostgres=# \r\rpostgres=# \\r\r\nQuery buffer reset (cleared).\r\npostgres=# " +# Did not match "(?^:CREATE COLLATION c FROM )" +not ok 111 - complete CREATE TYPE inside CREATE SCHEMA +# Actual output was "CREATE SCHEMA s CREATE TYPE t AS EN\r\rpostgres=# \r\rpostgres=# \\r\r\nQuery buffer reset (cleared).\r\npostgres=# " +# Did not match "(?^:CREATE TYPE t AS ENUM )" +``` + +Nothing is offered: the tab is inert. + +**What a correct implementation produces, and why.** The byte-identical tails complete at +top level, and the two families d516974 *did* convert complete inside `CREATE SCHEMA`. All +four are passing subtests in the same run: `ok 102 - complete CREATE DOMAIN inside CREATE +SCHEMA`, `ok 104 - complete CREATE TEXT SEARCH inside CREATE SCHEMA`, +`ok 112 - complete CREATE COLLATION at top level`, +`ok 114 - complete CREATE TYPE at top level`; plus +`ok 106`/`ok 108`, which show psql offering `COLLATION` and `TYPE` as clauses in the first +place (`/home/nm/src/pg/csxrun/evidence/CLIENT-O3-tap-new.txt`). + +**Mechanism** (against 6885b84). + +* `src/bin/psql/tab-complete.in.c:2199-2207` — the clause list, rewritten by d516974, now + offers `"AGGREGATE", "COLLATION", "DOMAIN", "FUNCTION", "INDEX", "OPERATOR", "PROCEDURE", + "SEQUENCE", "TABLE", "TEXT SEARCH ...", "TRIGGER", "TYPE", "VIEW"` (plus `"UNIQUE"` and + `"UNLOGGED"`, for `INDEX` and `TABLE`/`SEQUENCE` respectively). +* `tab-complete.in.c:3521` — `/* CREATE DOMAIN --- is allowed inside CREATE SCHEMA, so use + TailMatches */`, and `:3893` the same for `CREATE TEXT SEARCH`: both converted by d516974. +* `tab-complete.in.c:3493`, `:3495` — `Matches("CREATE", "COLLATION", MatchAny)` and + `Matches("CREATE", "COLLATION", MatchAny, "FROM")`: still anchored. +* `tab-complete.in.c:4163`, `:4165`, `:4174` — `Matches("CREATE", "TYPE", ...)`: still + anchored. `Matches()` requires `previous_words_count == narg`, which cannot hold once + `CREATE SCHEMA s` precedes. + +**Correction to the orchestrator's original statement of this finding.** It named +`CREATE FUNCTION`, `CREATE PROCEDURE`, `CREATE AGGREGATE` and `CREATE OPERATOR` as also +affected. They are not: an exhaustive grep of master's `tab-complete.in.c` for +`(Matches|TailMatches|HeadMatches)("CREATE", "FUNCTION|PROCEDURE|AGGREGATE|OPERATOR"` returns +nothing — psql has no completion rules for those four object types anywhere, inside or +outside `CREATE SCHEMA`. The defect is exactly `CREATE COLLATION` (3 rules) and +`CREATE TYPE` (6 rules). Of the nine, four (`:3497`, `:4167`, `:4176`, `:4190`) cannot be +rewritten as `TailMatches` at all — they anchor the head of an open parenthesised option +list, and psql's matcher vocabulary has no tail-anchored-head form. The actionable omission +is five one-word edits (`:3493`, `:3495`, `:4163`, `:4165`, `:4174`). + +**Rubric scores.** + +* `sql_mismatch = false` — and for the right reason. Collations and user-defined types *are* + `<schema element>`s in the standard (found.txt:25931-25953), but ISO/IEC 9075 has no + requirements about a client's interactive completion. On the server axis d516974 made + PostgreSQL strictly more conformant. +* `per_principles = true`, narrowly: the commit wrote the rule into the tree in its own diff + ("--- is allowed inside CREATE SCHEMA, so use TailMatches"), added `COLLATION` and `TYPE` to + the list of clauses it advertises, and did not apply the rule to them. That is an + internal-coherence gap in one commit's own diff. +* `is_new = true` for the two families, but see the counterarguments. + +**Counterarguments a reviewer should weigh — all of them verified, none of them refuted.** + +1. The invariant "everything psql offers as a clause completes inside `CREATE SCHEMA`" was + *never* universal. `CREATE TRIGGER` — one of the five clause keywords psql offered before + d516974 — goes dead inside `CREATE SCHEMA` from `ON <table>` onward, on `old`, `pre` and + master alike. Measured on `old` (`/home/nm/src/pg/csxrun/evidence/refute-C18-old-1.txt`): + `[CREATE SCHEMA s CREATE TRIGGER trg AFTER INSERT ON tab1 ]` + TAB TAB offers nothing, + while the same tail at top level offers + `DEFERRABLE FOR NOT DEFERRABLE WHEN ( EXECUTE FUNCTION INITIALLY REFERENCING`. +2. Un-anchoring is a documented, complaint-driven backlog. `tab-complete.in.c:20-22` says + the file "does not always give you all the syntactically legal completions"; commit + `9b181b0`, where head-anchoring became the default and where the very "so use TailMatches" + comment was written, says "we can put those back when we get complaints"; and `ef0938f` + (2024) *added* head-anchored `CREATE TRIGGER` rules under that comment. +3. The mechanism is generic, not CREATE SCHEMA-specific: `[BEGIN; CREATE TYPE t ]` + TAB is + equally dead, and `[BEGIN; CREATE COLLATION c FROM ]` + TAB offers + `information_schema. public. mytab123 tab1` — on `new`, `old` and `pre` alike + (`/home/nm/src/pg/csxrun/evidence/refute-C18-new-1.txt` lines 120-186). The + `COLLATION ... FROM` case offering non-collation names is therefore a pre-existing psql + behaviour and is **not** charged to d516974. + +**Attribution: d516974**, for the two families it advertised and did not convert. Cosmetic: +no crash, no wrong catalog contents, no silently-wrong result; a user who accepts a wrong +suggestion gets an immediate, accurate server error. + +**Test on this branch: yes.** `src/bin/psql/t/010_tab_completion.pl`, subtests 110-111 (of +115), body at lines 486-556, with six passing controls around them (102, 104, 106, 108, 112, +114). A new helper `check_completion_nofail()` at lines 129-150 is *required*, not cosmetic: +the file's own `check_completion()` waits `PG_TEST_TIMEOUT_DEFAULT` and then **dies**, which +would abort the file after two subtests +(`/home/nm/src/pg/csxrun/evidence/PLAN-o3-die-new.txt`). + +--- + +## 3. Complete failure set produced by this branch's tests + +The four tests above make **seven** meson tests fail, not four. Three of the seven are +collateral: `pg_upgrade/002_pg_upgrade`, `recovery/027_stream_regress` and +`test_plan_advice/001_replan_regress` each run the core `parallel_schedule`, so each fails +only on the shared `create_schema` test with a `regression.diffs` whose body is byte-identical +to `regress/regress`'s. That is inherent to any regress-file test encoding correct behaviour. + +From `/home/nm/src/pg/csxrun/evidence/FINAL-suiterun.txt`: + +``` +1/9 postgresql:psql / psql/020_cancel OK 1.32s 2 subtests passed +2/9 postgresql:psql / psql/030_pager OK 1.54s 5 subtests passed +3/9 postgresql:psql / psql/010_tab_completion ERROR 1.64s exit status 2 +4/9 postgresql:plpgsql / plpgsql/regress ERROR 3.98s exit status 1 +5/9 postgresql:psql / psql/001_basic ERROR 8.11s exit status 3 +6/9 postgresql:regress / regress/regress ERROR 87.69s exit status 1 +7/9 postgresql:test_plan_advice / test_plan_advice/001_replan_regress ERROR 90.62s exit status 1 +8/9 postgresql:recovery / recovery/027_stream_regress ERROR 95.70s exit status 1 +9/9 postgresql:pg_upgrade / pg_upgrade/002_pg_upgrade ERROR 99.04s exit status 1 +Ok: 2 +Expected Fail: 0 +Fail: 7 +``` + +| meson test | failing unit | finding | +|---|---|---| +| `regress/regress` | `create_schema`, 1 of 243 | D2, and S1 below | +| `plpgsql/regress` | `plpgsql_misc`, 1 of 13 | D3 | +| `psql/001_basic` | subtests 135-137 of 140 | D1 | +| `psql/010_tab_completion` | subtests 110-111 of 115 | D4 | +| `pg_upgrade/002_pg_upgrade` | subtest 5 "regression tests in old instance" | collateral | +| `recovery/027_stream_regress` | subtest 2 "regression tests pass" | collateral | +| `test_plan_advice/001_replan_regress` | subtest 1 "regression tests pass" | collateral | + +`pg_upgrade` subtest 20 ("old and new dumps match after pg_upgrade") and `recovery` subtests 7 +and 10 still pass, proving the added blocks leave no catalog residue. With the additions +stashed, all seven are green. Four of the five shipped tests were shown to flip to passing +under a throwaway fix (D1 by resetting sub-clause state on `grant`; D3 by porting psqlscan.l's +per-clause tracking to `pl_gram.y`; D4 by two `Matches` → `TailMatches` edits; D2 by a guc.c +flag, which removes the D2 hunk and leaves only S1's two); S1 was not fix-checked because a +fix is a redesign. + +--- + +## 4. Findings that belong to the sibling commit a9c350d, not to d516974 + +These are presented as **context**. Each is a real, user-visible defect in master, and one +of them (S2) is the most severe thing this audit found anywhere — but each reproduces on +`old` (= `d516974^`) with a pre-d516974-legal element, so d516974 did not cause it. Nothing +was found attributable to `404db8f`. + +The common cause: `a9c350d` deleted `setSchemaName()`, which used to write the new schema's +name into each element's `RangeVar->schemaname` +(`/home/nm/src/pg/csx-pre/src/backend/parser/parse_utilcmd.c:4521-4535`, called at +`:4450/4459/4472/4485/4494`), and replaced it with `checkSchemaNameRV()`, which only +*verifies* an explicitly written name and leaves NULL as NULL. From then on the new schema +reaches its own elements solely through a prepended `search_path` entry +(`schemacmds.c:165-178`). + +### S1 — `CREATE SCHEMA "$user"` receives none of its own elements + +`quote_identifier("$user")` yields `"$user"`; `SplitIdentifierString` strips the quotes; and +`preprocessNamespacePath` (`src/backend/catalog/namespace.c:4211`) compares +`strcmp(curname, "$user") == 0` and substitutes the schema named after `GetUserId()`. The +new schema therefore never enters the path it was supposed to head. + +```sql +CREATE SCHEMA "$user" CREATE TABLE t (a int); +SELECT relnamespace::regnamespace AS t_landed_in FROM pg_class WHERE relname = 't'; +``` + +Observed on `new` (`/home/nm/src/pg/csxrun/evidence/PLAN-oracle-new.txt`, +`replC08-clean-new.txt` Y1): `CREATE SCHEMA`, then `t_landed_in` = `public`, and the schema +that was named is left empty. Variants, all verified: with a schema named after the current +role present, the elements land *there*; with `search_path = pg_temp, public` a plain, +non-`TEMP` `CREATE TABLE` element comes out `relpersistence = 't'` in `pg_temp_0` and vanishes +at session end (`refC08-r7-new.txt`); with a genuinely empty path the statement dies +`ERROR: no schema has been selected to create in` for the schema it just created itself; +and a `CREATE OR REPLACE VIEW`/`FUNCTION` element silently overwrites the object of that name +in `public` (`refC08-r3-new.txt` §E). Self-contradiction: the *qualified* spelling +`CREATE SCHEMA "$user" CREATE TABLE public.tq (a int)` is rejected with +`ERROR: CREATE specifies a schema (public) different from the one being created ($user)` +while the unqualified spelling silently does exactly what the guard forbids +(`replC08-x5-new.txt` U1/U2). + +`sql_mismatch = true`: ISO/IEC 9075-2 5.4 SR 4)b) (found.txt:8701-8702) reads verbatim +"If the <local or schema qualified name> is contained in a <schema definition>, then the +<schema name> that is specified or implicit in the <schema definition> is implicit." With +`<table name> ::= <local or schema qualified name>` (found.txt:8509) and 11.1 GR 1)e) +(found.txt:26007), `CREATE SCHEMA "$user" CREATE TABLE t (a int)` shall create `"$user".t`. +Three scope limits: SR 4 covers only names *without* a qualifier; it reaches only object +kinds the standard has, so it must **not** be cited for the `CREATE INDEX`, aggregate, +operator or text-search instances; and 11.1 SR 6/SR 8 do **not** license the prepended path +as the *placement* mechanism, because SR 8 (found.txt:25984-25986) scopes the SQL-path to +"unqualified <routine name>s that are immediately contained in <routine invocation>s", not to +where an element's own name is created. + +`is_new = false`, decisively: `old` and `new` agree on every variant expressible in +pre-d516974 syntax, and `pre` is correct on all of them +(`adjC08-new.txt` / `adjC08-old.txt` / `adjC08-pre.txt`; `refC08-r3-{new,old,pre}.txt`). +`CREATE TABLE` alone reproduces the whole thing on `old`. Severity low, and the trigger is +narrower than it first appears: if a role literally named `$user` is the effective user, the +macro resolves to the new schema and everything is correct, so the `AUTHORIZATION`-derived +route — the only way such a schema plausibly arises without deliberate intent — is safe +(`refC08-r6-new.txt` §V/§V2/§V3). Exactly two schema names are affected (`$user` and +`pg_temp`); `a,b`, `a"b`, `PUBLIC`, `select`, `$USER`, `$user2`, ` $user`, `üser`, embedded +newlines and 63-byte names all land correctly (`refC08-r4-new.txt`). + +**A test for S1 is on this branch** because it is cheap and unambiguous, clearly labelled as +a9c350d's: `src/test/regress/{sql,expected}/create_schema.{sql,out}` block at +`sql/create_schema.sql:213-238`, hunks `@@ -376,14 +376,14 @@` and `@@ -392,13 +392,12 @@`. +Both halves run inside `BEGIN ... ROLLBACK` so that neither the buggy nor the fixed behaviour +leaves objects in another schema. The oracles are real: the first half's expected output is +master's own behaviour run as a role literally named `$user`; the second half's is build +`pre` (`/home/nm/src/pg/csxrun/evidence/PLAN-oracle3-pre.txt`). + +### S2 — the session temp schema outranks the new schema for unqualified names + +`finalNamespacePath` (`namespace.c:4291-4305`) sets `*firstNS` (the creation namespace) from +the explicit path, then `lcons()`es `myTempNamespace` and `pg_catalog` onto the *front* of +the lookup list. So `CREATE SCHEMA k CREATE TABLE s (y int) CREATE INDEX ON s (y)` indexes a +pre-existing *temp* table named `s` rather than the table the first element just created. +This is the most severe item in the audit: it reaches a genuine confused deputy, verified on +`old` (`/home/nm/src/pg/csxrun/evidence/adjC07-G-old.txt`), where an unprivileged caller with +a colliding temp table makes a superuser-owned SECURITY DEFINER `CREATE SCHEMA` attach a +trigger to their own temp table. It is nevertheless **not a d516974 finding and arguably not +a defect at all**: every symptom reproduces with `SET search_path` and no `CREATE SCHEMA` +anywhere, byte-identically on `new`, `old` and `pre` +(`ref-C07-ROUT-new.txt`, `ref-C07-RSD-{new,pre}.txt`), and both halves are documented. +`doc/src/sgml/config.sgml:10349-10353`: "If it is not listed in the path then it is searched +first (even before `pg_catalog`). However, the temporary schema is only searched for relation +(table, view, sequence, etc.) and data type names. It is never searched for function or +operator names." — one paragraph that predicts every symptom, including that functions and +operators are immune. And `doc/src/sgml/ref/create_function.sgml:800` prescribes "write +`pg_temp` ... as the last entry in `search_path`", which defeats the escalation entirely. +No test. + +### S3 — an unqualified relation reference in an index or trigger element binds through `search_path` + +`CREATE SCHEMA s CREATE INDEX ON public_tab (a)` puts the index in `public`. Verified on +master (`/home/nm/src/pg/csxrun/evidence/PLAN-oracle-new.txt`): `CREATE SCHEMA`, then + +``` + relname | nspname +----------------+--------- + cs_outside_idx | public + cs_outside_tab | public +``` + +Two limits on the standard argument. The repro's object type — an index — does not exist in +the standard at all, so 5.4 SR 4)b) cannot be cited for it. And where SR 4)b) does apply, the +name at issue is a *reference*, so the standard's outcome would be that the reference denotes +`<new schema>.public_tab` and the statement **errors** — not that it binds in `public`. +`IndexStmt` (`parse_utilcmd.c:4194`) and `CreateTrigStmt` (`:4203`) are the only two element +types whose `RangeVar` is a lookup rather than a creation name, and both are pre-d516974 +element types. a9c350d; no test (a test would have to invent a design decision). + +### S4 — user code run by an element can hijack the temporary `search_path` + +A plain (non-`LOCAL`) `SET search_path` executed by code an element invokes rewrites +`CreateSchemaCommand`'s own GUC stack entry from `GUC_SAVE` to `GUC_SET` +(`guc.c:2055-2065`), and `AtEOXact_GUC` restores only a `GUC_SAVE` entry (`guc.c:2212`), so `AtEOXact_GUC(true, save_nestlevel)` at `schemacmds.c:237` keeps the +user's value: the private path leaks into the session, and every element after the hijack is +created wherever the path now points, while the statement still reports `CREATE SCHEMA`. +Reachable on `old` with bare built-ins and pre-d516974 element types only — +`FOR VALUES IN (length(set_config('search_path','public',false)))` +(`/home/nm/src/pg/csxrun/evidence/ADJ-C10-C-old.txt`). `DefineIndex` guards against exactly +this (`indexcmds.c:1314-1319`, "Roll back any GUC changes executed by index functions, and keep subsequent changes local to this command"); +`CreateSchemaCommand` has no equivalent — but neither does `CREATE EXTENSION`, which uses the +byte-identical idiom (`extension.c:1321/1362/1503`) and exhibits both halves inside one +top-level statement on `pre` (`refute-C10.txt` TRANSCRIPT 5). a9c350d, at most; no test. + +### S5 — a forward reference inside one statement silently binds a different object + +Because the temporary path retains the caller's whole path, an element naming an object the +same statement defines *later* binds a same-named object from the caller's path, while a later +element binds the new one. Six instances measured on master (`H01-b-new.txt` G/H, +`H01-e-new.txt` O1/O2, `H01-h-new.txt` S1b), and demonstrable on `old` with a `CREATE VIEW` +element (`H01-c-*.txt` P5: binds `public.fwd` on new/old, `sfwd.fwd` on `pre`). a9c350d's own +commit message says such forward references "used to work and no longer will"; the accurate +statement is that they fail when nothing same-named is reachable and silently rebind when +something is. What d516974 adds is blast radius: a wrong binding can now be frozen into an +aggregate's `SFUNC`, an operator's implementation function, a TS dictionary's template or a +`BEGIN ATOMIC` body rather than only a view's rewrite rule. No test. + +--- + +## 5. Examined and found correct + +This is what makes the short findings list credible. Sixteen hunt lenses (H01-H16) and six +recon dossiers ran roughly 300 probe transcripts across the three builds; the entries below +are negative results, each backed by a cited transcript. The cassert server-log grep +(`PANIC|FATAL|TRAP|Assert|server closed|terminating`) was empty for every run. + +**Grammar and reachability.** All 21 newly-reachable statement productions were executed and +verified to reach execution (`R1-grammar-1.txt`, 26 cases d01-d18/f01-f06/n01-n02). Every command +name in the doc's clause list is reachable from `schema_stmt` and vice versa +(`H15-forms-new-1.txt`, 52 schemas created). `transformCreateSchemaStmtElements` handles +exactly 12 node tags and the grammar produces exactly 12, so its `default: elog(ERROR, +"unrecognized node type")` is unreachable — established by enumeration and by an empirical +sweep (`H09-f` F3). + +**Catalog contents, dependencies, ownership.** For every `CREATE TYPE` spelling, `pg_type` +(14 columns), `pg_range` and `pg_depend` are byte-identical inside vs outside `CREATE SCHEMA` +(`H05-i-new.txt` S2). A complete side-by-side of all newly-allowed object types created +inside `CREATE SCHEMA ... AUTHORIZATION regress_su` vs at top level under `SET ROLE regress_su` +matches on all 23 catalog rows and all 13 `pg_shdepend` `deptype='o'` entries +(`h02-h-new.txt` H3/H4). A `CREATE SCHEMA`-built domain records exactly the same dependency +set as the fully-qualified outside control (`h06-f-new.txt` F1). Shell operators created via +`COMMUTATOR`/`NEGATOR` land in the new schema, are owned by the `AUTHORIZATION` role, and +round-trip through pg_dump (`H07-b-1.txt`, `H07-d-1.txt`). + +**pg_dump / pg_restore / pg_upgrade.** 28 elements built two ways — one `CREATE SCHEMA` and +the same objects as separate commands under the same path — produce identical `pg_dump` +output after schema-name normalisation, and a full dump → fresh database → re-dump is +likewise identical, with a 92-row catalog census matching (`H13-dump-new-1.txt`). In both +comparisons the *only* differing lines are pg_dump's random `\restrict`/`\unrestrict` +nonce, which is regenerated per dump; the transcript labels those two diffs `DIFFERENT:` and +`ROUND TRIP DIFFERS:` for that reason alone. A hard round trip +with cross-referencing objects (`BEGIN ATOMIC` functions over a same-schema table and domain, +an aggregate over an atomic function, an operator over it, a view over both, a TS +parser+dictionary+configuration with a mapping, `COLLATION c2 FROM c1`, a range type with +`SUBTYPE_OPCLASS`/`COLLATION`/`MULTIRANGE_TYPE_NAME`, and a shell operator left by an +unresolvable `NEGATOR`) restores with zero errors and re-dumps identically +(`H13-hardrt-new-2.txt`). pg_dump never emits `CREATE SCHEMA` with elements, so nothing +round-trips through the new grammar. `pg_dump --binary-upgrade` → restore under `postgres -b` +gives 0 restore errors, and preset binary-upgrade OIDs are consumed by the right objects +inside `CREATE SCHEMA` exactly as outside (`H14-bu-1.txt`, `H14-bu-3.txt`). + +**Event triggers, deparse, hooks.** Every element type produces a +`pg_event_trigger_ddl_commands()` row for the element's own object, with byte-identical +`command_tag`, `object_type`, +fully-qualified `object_identity`, correct `schema_name` (including reporting `public` for an +element that lands in `public` via the S3 mechanism), `objsubid = 0`, and rows in source order +(`h10-p1-new`, `h10-p8-new`, `h10-p3-new`). Side-effect objects — array types, multiranges, +range constructors, implicit PK/unique indexes, domain constraints, the composite `pg_class` +row, self-commutator shells — are reported identically inside and outside (`h10-p2-new`). +`test_ddl_deparse` types and tags all 11 tested kinds correctly. `ProcessUtility_hook`, +`OAT_POST_CREATE` and `OAT_NAMESPACE_SEARCH` see every sub-command individually, nested inside +the `CREATE SCHEMA` (`h10-p5-new`). `in_extension` is correct for every element inside an +extension script (`h10-p4b-new`). + +**Extensions.** The member set for a schema built inside `CREATE SCHEMA` in an extension +script is byte-identical to the same objects built by top-level statements in the same script +(`diff` empty); `DROP EXTENSION` leaves 0 orphans and 0 dangling `pg_depend` rows; +`ALTER EXTENSION ... ADD/DROP` round-trips; `ALTER EXTENSION ... UPDATE` records 14 new +members; the script's `search_path` is correctly restored after the element block +(`H14-ext-1..5`). + +**GUC and identity handling.** `search_path` and `current_user` are restored after success, +after failure, after `ROLLBACK TO SAVEPOINT`, after a caught plpgsql `EXCEPTION`, and after +2000 caught failures in one `DO`; no "GUC nest level" warning was ever emitted +(`h03-a-new.txt`, `H16-j-new.txt`, `H16-r-new.txt`). Odd `search_path` values (`''`, a +nonexistent schema, embedded whitespace, quoting) behave. `SET LOCAL` inside an element is +correctly discarded. `681d9e4`'s `NewGUCNestLevel`/`GUC_ACTION_SAVE` mechanism nests +correctly for the new element types even under `SECURITY_RESTRICTED_OPERATION` + +`RestrictSearchPath()` — the CVE-2023-2454 shape was re-run for this commit's object types +inside `REFRESH MATERIALIZED VIEW` and is clean, with the restricted path intact immediately +after the `CREATE SCHEMA` returns and every object in its own new schema +(`h08-secrest-new.txt`). + +**Unusual outer contexts (E7).** 30 theories, all negative: plpgsql `EXECUTE` and static +statements, SQL-language function bodies, `SECURITY DEFINER`, procedure transaction control, +read-only SPI, savepoints, `PREPARE TRANSACTION` (an 11-element body prepares, commits and is +fully usable), concurrent duplicate schema creation, lock timeouts mid-list, binary-upgrade +mode, `debug_discard_caches = 1`, row triggers, event-trigger functions, extended query +protocol, `BEGIN READ ONLY`, `SERIALIZABLE`, and single-user mode (`H14-ctx-*`, `H14-2pc-1`, +`H14-bu-*`). + +**Diagnostics.** 73 adversarial statements covering every newly-allowed element type, each +run inside `CREATE SCHEMA` and again at top level under an equivalent `search_path`, produced +**0 differing diagnostics** (`H16-sweep-new.txt`). Error positions that are emitted are +correct, including at byte offset ~215 000 for element #8001 (`H16-s-new.txt` S3) and inside +an extension script, plpgsql static/`EXECUTE`, SQL functions and mid-way through a +multi-statement simple query (`H14-pos-1.txt`). + +**Robustness.** 4000 `CREATE DOMAIN` elements take 0.13 s with `MessageContext` flat at +64 KB; 100 000 elements ends with a clean "out of shared memory" hint; a 4000-deep domain +chain works; 100-level recursive `CREATE SCHEMA` restores outer state; self-referential +definitions all give ordinary errors; 1700-column composites and 101-argument functions give +the documented limits identically inside and outside; a 10 000-label enum and a 1 MB function +body are fine (`H16-d2/k/a-1/r-new.txt`). + +**Client tools.** ecpg preprocesses, compiles and runs a 28-object `CREATE SCHEMA` correctly; +its two limitations (`$n` inside a dollar-quoted body; a host variable in a DDL element) +reproduce identically outside `CREATE SCHEMA`. Independently re-verified this pass: 14 +element shapes preprocessed twice each (as a clause and standalone) all have `in == out`; the +two failures (`BEGIN ATOMIC` bodies; a `###` operator name taken as a C preprocessor +directive) fire identically outside (`CLIENT-ecpg-verify-new.txt`). psql's `\d`-family +describe commands are byte-identical for an object made by an element vs by a standalone +statement — `\df \da \do \dT \dD \dO \dF \dFd \dFp \dFt \d` and their `+` forms +(`H12-desc-1-new.txt`). psql `--single-transaction`, `-c`, `\i`, `\ir`, `:var` +interpolation, `\gexec`, `\if/\else/\endif`, `\g`, comments, dollar quotes and `COPY FROM +STDIN` after a correctly-split `CREATE SCHEMA` all behave (`H11-*`, `H13-psqlmodes-new-1.txt`). + +**Composition (checked by the completeness critic, since no lens owned the seam).** A trigger +function and its trigger in the same statement bind correctly; views, column `DEFAULT`s and +`CHECK`s over sibling functions all deparse qualified; a forward reference from a trigger +element to a later function element binds `public` — but so does the documented +separate-commands equivalent, so it is symmetric. A sibling plpgsql trigger function whose +unqualified `INSERT` writes to `public.audit_log` is likewise symmetric with the documented +equivalent (`CRIT-2-compose-new.txt`, `CRIT-2b-trigger-new.txt`). + +**Bounded negatives worth recording.** Non-core PL validators run arbitrary code at DDL time +inside the `CREATE SCHEMA` environment (a Perl `BEGIN` block fires during +`plperl_validator`), but the channel is closed one layer down: PL/Perl refuses SPI during +compilation, so that code cannot read `search_path`/`current_user`, cannot `SET search_path` +to redirect later elements, and cannot `DROP SCHEMA` the schema being created — all three +die with `ERROR: SPI functions can not be used during function compilation` and the whole +statement rolls back (`CRIT-1-plperl-new.txt`). And +`information_schema.domains.domain_default`'s frozen deparse (see §6, C09) is the *only* +instance of that class in `information_schema`: `routines.routine_definition` is raw `prosrc` +(`information_schema.sql:1589`), not a deparse at all, while +`parameters.parameter_default` uses `pg_get_function_arg_default` and +`attributes.attribute_default` uses `pg_get_expr`, both of which deparse at read time +(`CRIT-4b-infoschema-new.txt` plus `information_schema.sql:294/1022/1188`). + +--- + +## 6. Candidates investigated and rejected + +Nineteen candidates went through hunt → replicate → adjudicate → refute. Sixteen were +rejected. This is the most useful section for judging how much to trust the four that +survived: in almost every case the killer was the same control — *run the same thing outside +`CREATE SCHEMA`, or on `pre`, and watch it fail identically*. + +| id | candidate | why rejected | +|---|---|---| +| C01 | Text-bodied routine element validated against the temporary path: accepted where the documented equivalent is rejected, then dead or silently wrong | Every symptom reproduces with a plain `SET search_path` and no `CREATE SCHEMA` (`refute-C01-p1-new.txt` T1-T6, T9), including the "strongest exhibit" — identical bodies binding differently for `BEGIN ATOMIC` vs quoted (T5). It is documented, intended behaviour of `AS '<text>'` bodies (`create_function.sgml:614-627`). The same server-manufactured invisible path has shipped in `CREATE EXTENSION` since 2011 (`extension.c:1362`) and produces all three symptoms on `pre` (`refute-C01-ext-pre.txt`). Two premises were false: the "accepted inside, rejected outside" asymmetry already holds on `old`/`pre` for `CREATE VIEW`, and the pre-d516974 element set is *not* entirely early-binding (a `nextval('sq'::text::regclass)` default is stored as a name and dies in a fresh session on `old` and `pre`). | +| C02 | Aggregate `INITCOND`/`MINITCOND` validated under the temporary path and stored as raw text: unrestorable dump, pg_upgrade failure after `--check` said compatible | Reproduced with **no `CREATE SCHEMA`, the default `search_path`, and a fully qualified `INITCOND`** on `old`: `pg_dump` emits the aggregate before the table it names, restore loses it silently (`surviving_user_aggregates = 0`, psql exit 0), `pg_upgrade --check` says "Clusters are compatible" and the real run fails (`refuteC02-C-old.txt`). Inside and outside are indistinguishable down to `pg_depend` and pg_dump bytes (`refuteC02-D-new.txt` N1). The doc's own "equivalent way" qualifies internal cross-references; under the doc's actual recipe inside and outside agree exactly (`refuteC02-E-doc.txt`). | +| C04 | Thesaurus sub-dictionary named in an element stored as unqualified raw text: dead outside the statement, retargetable, lost by pg_dump | The raw-text storage, the missing dependency, the retargetability and the pg_dump gap are all properties of `CREATE TEXT SEARCH DICTIONARY` itself: the whole chain reproduces on `pre` with nothing but `SET search_path = z, public` (`repl-C04-attr-pre.txt`). What is left attributable to d516974 is the loss of one create-time error message in one narrow shape, and the first symptom a user sees is a loud, immediate `text search dictionary "..." does not exist` on the very next statement. | +| C05 | `checkSchemaNameList` guards only the element's own name, so elements create objects in other schemas including `pg_catalog` and `pg_temp` | The same statements standalone on master give byte-identical placement; the unqualified case is actually *better* inside. On `old`, pre-existing element types do everything the candidate calls new, and worse: `CREATE VIEW` over a temp table silently relocates the element's own object into `pg_temp`; a `CREATE TABLE` with a `pg_temp`-schema column type leaves a permanent table that loses its only column at session end. The headline damage (a permanent range type destroyed at session end) reproduces on `pre` with a standalone `CREATE TYPE`. | +| C06 | An element can drop the schema being created; `CREATE SCHEMA` still reports success and `pg_dump` then fails for the whole database | The documented separate-commands form gives a byte-identical outcome in the same session, and reproduces verbatim on `old` and `pre`. It needs no aggregate, no `INITCOND`, no domain and no user code: two ordinary concurrent sessions (`BEGIN; CREATE FUNCTION s.f() ...` / `DROP SCHEMA s;` / `COMMIT;`) orphan a `pg_proc` row on `pre` and break `pg_dump` for the database (`refC06-B-pre.txt`). It is a 14-year-old explicitly-deferred gap: `1575fbc` (2012) fixed relations only and says "We need similar protection for all other object types ... I'm leaving that for a separate commit." | +| C07 | Session temp namespace outranks the new schema (see §4 S2) | Identical outside `CREATE SCHEMA`, identical on `pre`, and documented twice (`config.sgml`, `create_function.sgml`). a9c350d at most; kept in §4 as context. | +| C09 | Domain `DEFAULT` deparsed under the temporary path and stored unqualified, so `information_schema.domains` and `\dD` report a default meaning a different function | Reproduces with the plainest top-level `CREATE DOMAIN public.dd AS int DEFAULT f();` under the default path, byte-identically on `new` and `pre`, and every ordinary domain's `domain_default` is likewise not dump/restore-stable. Documented as known since `3666260` (2006) — "default values for domains really need to be dumped by decompiling the typdefaultbin expression, not just printing the typdefault text which may be out-of-date or assume the wrong schema search path" — and the sibling `adsrc`/`consrc` columns were deleted for the same reason in `fe50382`. The rule has exactly one input (is the referenced object visible in the live path), so "inside stores the wrong text, outside the right one" is false in both directions. | +| C10 | `CreateSchemaCommand` does not defend its temporary path against user code (see §4 S4) | Full misplacement plus full leak on `old` with only built-ins and pre-d516974 element types; `CREATE EXTENSION` exhibits both halves on `pre` with a bare `SET` and no function at all. a9c350d at most. | +| C11 | Unqualified names resolve differently from the documented equivalent in both directions | Part (a) is a9c350d (a VIEW/TABLE-only script gives the same wrong binding on `old`, the right one on `pre`); parts (b) and (c) are identical on all three builds. The doc's own "equivalent way" form and the prepended path outside `CREATE SCHEMA` produce the *same* binding as inside. | +| C12 | Every `role`/`session_authorization` `SET`/`RESET` clause rejected under `AUTHORIZATION` (overlaps D2) | Not rejected as a phenomenon — it is D2 — but rejected as an *independent* candidate, because its own probes established that the rule violation predates the series: `adjC12-a3-pre.txt` reaches the byte-identical false message on `pre` using only pre-d516974-legal elements, and `adjC12-a4-pre.txt` reaches it with no `CREATE SCHEMA` at all inside a genuine SECURITY DEFINER function. §D2 carries the finding with that caveat stated. | +| C13 | Shell/base types and TS parser/template cannot be created under `AUTHORIZATION <non-superuser>` even when the invoker is a superuser | Holding the effective user fixed — the one thing `AUTHORIZATION` is defined to change — the behaviour is byte-identical to top level under `SET ROLE`. 11 of 16 probed element forms succeed under `AUTHORIZATION regress_bob`; the 5 that fail do so through each command's own long-standing superuser gate (`typecmds.c:221`, `tsearchcmds.c:197/704`, `functioncmds.c:1162`), none of which d516974 touches. On `pre` a superuser was already blocked from two of the six then-allowed element types by naming a non-superuser in `AUTHORIZATION`. What survives is a documentation nit only. | +| C15 | `DefineRange` multirange naming: an internal `elog` (XX000) and a raw unique-index violation reach the user; constructors land in the wrong schema | Real, but pre-existing (`6df7a96`, v14) and not about `CREATE SCHEMA`: a four-case probe that never uses `CREATE SCHEMA` is byte-identical on `new`, `old` and `pre`, and one statement with zero setup on `pre` produces `ERROR: 23505: duplicate key value ... (_r, 2200)`. Decisively, three of the four sub-observations require the two namespaces to differ, which *cannot* happen through `CREATE SCHEMA`'s own environment — an unqualified `MULTIRANGE_TYPE_NAME` resolves through the prepended path into the new schema. The cross-schema case is exercised by a shipped, passing in-tree test added five weeks *after* d516974 by the CVE-2026-6472 fix `4793fc4`. | +| C16 | Tag-filtered event triggers are silently bypassed by wrapping a command in `CREATE SCHEMA` | The same bypass occurs with no `CREATE SCHEMA` anywhere, identically on all three builds: a `WHEN TAG IN ('CREATE SEQUENCE')` deny trigger does not fire for `CREATE TABLE t (a serial)`, and a `CREATE INDEX` deny trigger does not fire for `CREATE TABLE ... PRIMARY KEY`. The invariant is "tag-filtered `ddl_command_start` has never fired for nested DDL", from `5525e6c` (2013). PostgreSQL's own documented DDL-gating recipe (an *unfiltered* start trigger) still works, and at `ddl_command_end` nothing is hidden. Residue: a documentation patch to `doc/src/sgml/event-trigger.sgml:89-101`, whose exception list for `ddl_command_start` names only shared objects and event-trigger commands. | +| C17 | The wrong-schema error for 20 of the 21 added forms has no `LINE`/caret | Outside `CREATE SCHEMA` these same commands are equally caret-less — only `CREATE TABLE` gets one at top level — so the documented equivalence is satisfied, and for three forms the inside diagnostic is strictly *better*. The commit states the limitation in its own comment (`parse_utilcmd.c:4323`, "Sadly, this also means we don't have a parse location to report") and its own regression test records the caret-less output as expected. Diagnostic-quality limitation, self-acknowledged; cosmetic. | +| C18 | Tab completion, superset of D4 | The parts that survived are §D4; the rest was refuted (see D4's counterarguments): the invariant was never universal, un-anchoring is a documented backlog, and the "misleading table list" is generic pre-existing psql behaviour. | +| C19 | `create_schema.sgml`'s "all the created objects will be owned by that user" is false for two newly added element types | The premise "before d516974 the claim held for every legal element type" is false: on `old`, `CREATE SCHEMA idxs1 AUTHORIZATION aj_bob CREATE INDEX carol_idx1 ON carol_tab (a)` produces an index owned by `aj_carol`, and on `pre` the sentence is already false via `CHECK` constraints and triggers. `CREATE TYPE ... AS RANGE` produces byte-identical catalogs inside and outside. Longstanding doc imprecision (the sentence is 3450fd08/2003), not this commit's. | +| O2 | A `PGC_SUSET` GUC in a routine element's `SET` clause is checked against the `AUTHORIZATION` role, not the invoking superuser | **Adjudicated intended.** `CREATE FUNCTION topg1() ... SET log_min_duration_statement = 1` issued at top level *by that same non-superuser role* fails with the identical `ERROR: permission denied to set parameter "log_min_duration_statement"` (`PLAN-o2ctl-new.txt`), and matches `SET ROLE` in all four sub-cases including after `GRANT SET ON PARAMETER` (`h02-d-new.txt` D2). Since the documented contract is "objects will be owned by that user", checking against that user is coherent; there is no output a correct implementation demonstrably ought to produce, so no test was written. Contrast D2, where the top-level control *succeeds* for the same role. | +| O4 | Unqualified `CREATE INDEX`/`CREATE TRIGGER` element binds outside the new schema | Behaviour confirmed, but "correct" is undecidable without inventing a design (error? bind in the new schema? create the index in the new schema over a `public` table?), and it is a9c350d's, sharing S1's mechanism. Reported as §4 S3; no test. | + +--- + +## 7. Limitations — what was not examined + +Taken from the completeness critic's pass; each item is a real gap in this audit. + +**Newly-allowed command forms.** At the grammar-production level, none was skipped: all 21 +were executed, and every variant the coverage dossier listed as untested was later run by some +lens (prefix/unary operators, pre-8.2 aggregate syntax, ordered-set / hypothetical-set / +moving aggregates, `RETURNS TABLE`, the no-`RETURNS` OUT-parameter form, `WINDOW`, +`TRANSFORM FOR TYPE`, `SUPPORT`, `LANGUAGE c` against a real shared library, all four +`CREATE COLLATION` forms, `CREATE TYPE ... (LIKE=...)`, `CANONICAL`, the full domain +`ColQualList`). Below production level, one gap survived to the critic and was then closed: +no lens had used a non-core procedural language as an element `LANGUAGE` (plperl/plpython/pltcl +*are* installed, contrary to one lens's stated gap) — closed, negative (§5). + +**Environment properties E1-E7.** E1, E2 and E7 are saturated. E3's second consequence +(`isTopLevel` gating `PreventInTransactionBlock`) was closed by grep, not by a probe: no +newly-allowed command calls it, so the only `isTopLevel`-sensitive element is the pre-existing +`CREATE INDEX CONCURRENTLY`. E4's `canSetTag = false` and `planOrigin = PLAN_STMT_INTERNAL` +were probed by nobody and closed by source reading: `planOrigin` has no in-core consumer, and +the only out-of-core one (`pg_stat_statements.c:1487-1489`) tests only for +`PLAN_STMT_CACHE_GENERIC`/`CUSTOM`. Four further E2/E7 sub-paths were answered only by +reading source: `GucStack.srole` (not exposed to SQL); `push_old_value`'s +`Assert(stack->state == GUC_SAVE)` for `GUC_ACTION_SAVE` (no input reaches it); whether any +user-sensitive GUC *assign* hook can hold a stack entry at `CreateSchemaCommand`'s nest level; +and whether a subtransaction opened inside an element leaves the GUC nest level inconsistent. + +**Not exercised at all.** +* `sepgsql` — not built (no `--with-selinux`); `test_oat_hooks` was substituted, so + `OAT_POST_CREATE` for the ~15 new object types under E1's identity switch is unexercised by + a real MAC module. +* An `InvokeNamespaceSearchHook` returning false for the new schema (`namespace.c:4281`). This + is the one code path that would silently relocate the creation namespace — S1's failure mode + by a second route, with no `$user` name required. Nobody wrote the C hook. +* Any C-level SPI caller (`worker_spi.c:107` is the only in-tree SPI `CREATE SCHEMA` with + elements, and it was never run on any build). +* `table_rewrite` event triggers (no allowed element can rewrite a table) and + `session_replication_role = replica` with `ENABLE REPLICA` triggers. +* psql `\copy`, `\e`, `\ef`, `\ev`. `\e` re-feeds the edited buffer through `psql_scan`, which + is D1's wedge with an editor in the loop. +* DDL-deparse consumers: no in-core deparser exists, so nobody could observe what an extension + deparsing a stashed element parse tree would emit. `checkSchemaNameRV`/`checkSchemaNameList` + only *validate*, so the stashed tree carries an unqualified name whose meaning depends on the + temporary path. +* The interactive-terminal consequence of D1 was found only in the completeness pass; it did + **not** go through the refute stage, and it was not run on `pre`. + +**Structural weaknesses of the audit itself, stated so they can be discounted.** Ten of the +sixteen lenses (H01, H02, H05, H07, H08, H09, H11, H12, H13, H15) wrote no coverage-gaps +section at all — including the three that produced the most surviving material. Three of the +six that did wrote gaps that were factually wrong about the environment (claiming +`pg_stat_statements` unavailable when another lens had used it; claiming plperl/plpython not +built when they are installed; claiming no way to run arbitrary user code at DDL time using +only the new element types, when a plperl validator does exactly that). Lens boundaries were +drawn per object type, so "new element × old element" compositions were owned by nobody until +the completeness pass ran them (negative). Finally, the `new` install tree is contaminated: +`/home/nm/src/pg/csx-inst/share/postgresql/extension/` still holds four lenses' leftover +extensions (`csxfail`, `csxg`, `csxh05`, `csxok`), so any future probe enumerating +`pg_available_extensions` on `new` will see them. + +--- + +## 8. How to reproduce everything + +### Builds + +``` +/home/nm/src/pg/csx-audit 6885b845b4ba master, this audit branch (create-schema-defect-tests) +/home/nm/src/pg/csx-old 404db8f9edbb d516974~1 +/home/nm/src/pg/csx-pre 1ff3180ca016 a9c350d~1 +``` + +Installed trees: `/home/nm/src/pg/csx-inst`, `csx-old-inst`, `csx-pre-inst`. +Build dirs: `/home/nm/src/pg/csx-build`, `csx-old-build`, `csx-pre-build`. + +### Running a probe on any build + +``` +/home/nm/src/pg/csxrun/pgrun.sh <new|old|pre> file.sql [more.sql ...] # SQL +/home/nm/src/pg/csxrun/pgsh.sh <new|old|pre> script.sh # shell +``` + +Each spins up a throwaway cluster (`initdb -A trust`, `fsync=off`), runs the input with +`psql -X -e -v ON_ERROR_STOP=0`, prints the output, destroys the cluster, and greps the server +log for `PANIC|FATAL|TRAP|Assert`. Extra server options via `PGRUN_OPTS` (e.g. +`PGRUN_OPTS="-c log_statement=all"`), extra initdb options via `PGRUN_INITDB_OPTS`. +`pgsh.sh` exports `BIN`, `PGHOST`, `PGUSER`, `PGDATABASE` and puts `BIN` on `PATH`. +**Always use `psql -X`**: the repository owner's `~/.psqlrc` enables per-statement rollback, +which masks transaction-abort behaviour and would mislead on D1 in particular. + +### The four findings, one command each + +``` +# D1 (psql lexer) -- expect: VACUUM error, exit 3, cs_gb absent; oracle and control clean +/home/nm/src/pg/csxrun/pgsh.sh new /home/nm/src/pg/csxrun/probe/client-c03/oracle.sh +# D1 discriminator vs the case 049b742 deferred +/home/nm/src/pg/csxrun/pgsh.sh new /home/nm/src/pg/csxrun/probe/xprem/p6.sh +# D2 (SET role under AUTHORIZATION) +/home/nm/src/pg/csxrun/pgrun.sh new /home/nm/src/pg/csxrun/probe/plan/o1o2.sql +/home/nm/src/pg/csxrun/pgrun.sh pre /home/nm/src/pg/csxrun/probe/adjC12/a3.sql # attribution +# D3 (PL/pgSQL) -- run on all three builds +/home/nm/src/pg/csxrun/pgrun.sh new /home/nm/src/pg/csxrun/probe/adjC14/adj.sql +/home/nm/src/pg/csxrun/pgrun.sh old /home/nm/src/pg/csxrun/probe/adjC14/adj.sql +# D4 (tab completion) -- needs a pty; see the shipped TAP test below +# S1 (a9c350d, "$user") +/home/nm/src/pg/csxrun/pgrun.sh new /home/nm/src/pg/csxrun/probe/plan/oracle.sql +/home/nm/src/pg/csxrun/pgrun.sh pre /home/nm/src/pg/csxrun/probe/plan/oracle3.sql +``` + +### The shipped tests + +Six files changed on branch `create-schema-defect-tests`, +339/-0, all test inputs or expected +files; no compiled source is touched, so **no rebuild is needed** +(`ninja -C /home/nm/src/pg/csx-build -n` reports "no work to do"): + +``` +src/test/regress/sql/create_schema.sql src/test/regress/expected/create_schema.out +src/pl/plpgsql/src/sql/plpgsql_misc.sql src/pl/plpgsql/src/expected/plpgsql_misc.out +src/bin/psql/t/001_basic.pl src/bin/psql/t/010_tab_completion.pl +``` + +``` +meson test -C /home/nm/src/pg/csx-build --suite setup +meson test -C /home/nm/src/pg/csx-build regress/regress plpgsql/regress \ + psql/001_basic psql/010_tab_completion psql/020_cancel psql/030_pager \ + pg_upgrade/002_pg_upgrade recovery/027_stream_regress \ + test_plan_advice/001_replan_regress --print-errorlogs +``` + +Expected: exactly the seven failures tabulated in §3 and nothing else. Diffs land in +`/home/nm/src/pg/csx-build/testrun/<group>/<name>/regression.diffs`; TAP output in the +corresponding `log/regress_log_*`. Every added expected-output block records what a +**correct** implementation would print, so the failure diff *is* the demonstration. + +To drive one regress test standalone (avoiding the schedule): + +``` +B=/home/nm/src/pg/csx-build; S=/home/nm/src/pg/csx-audit/src/test/regress +export PATH="$B/tmp_install/home/nm/src/pg/csx-inst/bin:$B/src/test/regress:$PATH" +export LD_LIBRARY_PATH="$B/tmp_install/home/nm/src/pg/csx-inst/lib/x86_64-linux-gnu" +export INITDB_TEMPLATE="$B/tmp_install/initdb-template" +"$B/src/test/regress/pg_regress" --inputdir="$S" --expecteddir="$S" --bindir= \ + --dlpath="$B/src/test/regress" --dbname=regression --outputdir=/tmp/csxrun \ + --temp-instance=/tmp/csxrun/tmp_check --port=48460 create_schema +``` + +(for plpgsql: `--inputdir`/`--expecteddir` `/home/nm/src/pg/csx-audit/src/pl/plpgsql/src`, +`--dlpath "$B/src/pl/plpgsql/src"`, `--dbname regression_plpgsql`, test `plpgsql_misc`). + +### Evidence + +Every transcript this report quotes is under `/home/nm/src/pg/csxrun/evidence/`; the probe +sources that produced them are under `/home/nm/src/pg/csxrun/probe/`. The per-lens working +notes, including the full theory lists and negative results summarised in §5, are under +`/home/nm/src/pg/csxrun/notes/` (`H01`-`H16`, plus recon dossiers `R1`-`R6`). Key files: + +| finding | evidence | +|---|---| +| D1 | `CLIENT-C03-oracle-new.txt`, `CRIT-3-interactive-{new,old}.txt`, `XPREM-p6-new.txt`, `ADJ-C03-A-{new,old}.txt`, `refute-C03-run{1,2,3,4}*.txt`, `FINAL-tap.txt`, `FACTCHECK-D1-{oracle,xprem6,pty,var}-new.txt` | +| D2 | `PLAN-o1o2-new.txt`, `PLAN-o2ctl-new.txt`, `PLAN-oracle-new.txt`, `VERIFY-o1-mechanism-new.txt`, `h02-a-{new,old,pre}.txt`, `adjC12-a3-pre.txt`, `FINAL-regressdiffs.txt`, `FACTCHECK-D2-{spellings,o1o2}-new.txt`, `FACTCHECK-D2-a3-pre.txt` | +| D3 | `adjC14-{new,old,pre}-1.txt`, `PLAN-oracle4-new.txt`, `refC14-r{1,2,3,4}*.txt`, `XPREM-p{2,4,5}*.txt`, `FINAL-regressdiffs.txt`, `FACTCHECK-D3-{new,old}.txt` | +| D4 | `CLIENT-O3-tap-new.txt`, `PLAN-o3-die-new.txt`, `refute-C18-{new,old,pre}-*.txt`, `FINAL-tap.txt` | +| S1 | `PLAN-oracle-new.txt`, `PLAN-oracle3-pre.txt`, `adjC08-{new,old,pre}.txt`, `refC08-r{3..8}*.txt`, `replC08-*.txt` | +| test run | `FINAL-suiterun.txt`, `FINAL-regressdiffs.txt`, `FINAL-tap.txt`, `FINAL-gitdiff.txt`, `VERIFY-*`, `FACTCHECK-suiterun.txt` | diff --git a/PROVENANCE.md b/PROVENANCE.md new file mode 100644 index 0000000..19b314d --- /dev/null +++ b/PROVENANCE.md @@ -0,0 +1,305 @@ +# PROVENANCE + +Branch `create-schema-defect-tests` is the product of an automated, model-driven audit of +PostgreSQL commit `d516974` ("Support more object types within CREATE SCHEMA."). Everything +on it -- the report and the regression tests -- was written by a language model. **No human +wrote any of it, and no human has reviewed it.** This file records how it was produced so +that a reviewer can judge the work and reproduce the verification. + +--- + +## Tooling + +* **Tool:** Claude Code (Anthropic's agentic CLI), using its Workflow orchestration feature + (one deterministic script driving many subagents). +* **Model:** Opus 5 (`claude-opus-5`), for the orchestrator and for every subagent. +* **Run by:** the repository owner (Noah Misch), interactively, from + `/home/nm/src/pg/postgresql`. +* **Workflow run ID:** `wf_784c5f4f-e9b`. Started 2026-09-02 23:19 UTC; relaunched + 23:33 UTC after the orchestrator rewrote the scheduler (see [Scheduling](#scheduling)). +* **Human contribution:** the two prompts quoted below, and nothing else. The owner supplied + no finding, no verdict, and no line of any test. + +## The prompt + +Verbatim, as typed by the repository owner: + +``` +Make a large workflow, with at most 90 agents, to write test cases covering +user-visible defects in commit d516974 that are still present in master. Use +your own worktree; disregard the present dir except as repository to which to +attach your worktree. The workflow should first look for extant user-visible +defects. If it finds any, write a test case covering some of those defects. +If any defects found weren't suitable to test, describe them in a report. + +The workflow should manage budget so, when it reaches a budget stop, it loses +1-2 agents instead of 16 concurrent agents. + +Commit the following on a fresh branch: +- A report describing any defects found, testable or not. Prefix the report + with [no defects] if that's so. +- Any tests written +- A PROVENANCE.md file containing model, prompt, etc. + +For this one, I'm wary that CREATE SCHEMA is a weird execution environment. +I'm reminded of CVE-2023-2454 resulting from the weird environment. Commit +d516974 means a bunch more old code needs to be ready for that weird +environment. (Granted, the CVE-2023-2454 fix made the environment less weird. +If we're lucky, all is now well.) +``` + +A second message corrected the first implementation of the budget requirement: + +``` +That's a terrible implementation of the budget goal. Now I won't even use my +budget before it expires. If you can't do better than that, just run full 16 +concurrency and accept the loss. + +I wanted something like: start at 16 concurrent, but when one exits, estimate +the budget burndown curve. If there's a decent chance one started now would +fail due to budget exhaustion, don't replace the one that just exited. +``` + +## Environment + +* **Base commit:** `6885b845b4ba0b7aee09daa9817703477faa3704` -- "doc: Fix link on + pg_dsm_registry_allocations page." This branch is based on it; every `file:line` + citation in the report is against it. +* **Audited commit:** `d516974840f4059d331ae6057ede3e4edd3c6747` -- "Support more object + types within CREATE SCHEMA." (Kirill Reshke, Jian He, Tom Lane; 2026-04-06). +* **Worktree:** `/home/nm/src/pg/csx-audit`, a `git worktree` attached to the owner's clone + at `/home/nm/src/pg/postgresql`. +* **Builds**, all meson, all `-Dcassert=true -Ddebug=true -Doptimization=1 + -Dtap_tests=enabled`: + + | name | source | purpose | + |---|---|---| + | `new` | `6885b84` (this worktree) | every "master" observation in the report | + | `old` | `404db8f` = `d516974~1` | the `is_new` dimension for d516974 itself | + | `pre` | `1ff3180` = `a9c350d~1` | before the whole CREATE SCHEMA series, so findings can be attributed to d516974 rather than to its two same-day siblings a9c350d and 404db8f | + +* **Harness:** `/home/nm/src/pg/csxrun/pgrun.sh <new|old|pre> file.sql ...` initdb's a + throwaway cluster under `/tmp`, starts it with `fsync=off` and a unix socket only, runs each + file with `psql -X -e -v ON_ERROR_STOP=0`, prints the transcript plus any PANIC/FATAL/TRAP + lines from the server log, then destroys the cluster. `pgsh.sh <new|old|pre> script.sh` + does the same but runs an arbitrary shell script with `BIN`, `PGHOST`, `PGUSER`, + `PGDATABASE` exported, for pg_dump round-trips, ecpg, and psql-lexer tests. +* **SQL standard text:** `/home/nm/src/pg/gbyrun/std/found.txt`, pdftotext output of + `5CD2-02-Foundation-2006-01.pdf` (the 2006-01 committee draft of ISO/IEC 9075-2, which has + the General Rules). 11.1 `<schema definition>` begins near line 25895. +* **Host:** Debian 13 (trixie), Linux 6.12.94 x86_64, gcc (Debian 14.2.0-19) 14.2.0, + meson 1.7.0, ninja 1.12.1, Python 3.13.5. Two effective CPUs (cpuset cgroup). +* **Not part of the deliverable:** `/home/nm/src/pg/csxrun/` (recon dossiers, per-lens notes, + every raw transcript) and the build and install trees. Nothing on this branch depends on + them except the reproduction commands in the report, which a reviewer would re-create. + +--- + +## The rubric + +The owner had corrected an earlier audit for reporting *changes* rather than *defects* +("You're not here to be a change detector, you're here to find defects"), and gave this +rule, which was written into the prompt of every agent in this run: + +Each candidate is scored on three independent dimensions: + +* `sql_mismatch` -- the SQL standard requires behavior master does not exhibit; +* `per_principles` -- the commit's own principles (its commit message, the code comments it + wrote, the documentation it wrote, and the proposition that a command documented as + accepted inside CREATE SCHEMA should behave there as it does outside, apart from landing in + the new schema) require behavior master does not exhibit; +* `is_new` -- the RULE VIOLATION, not the syntax, is new relative to the state before + d516974. + +**Defect iff `per_principles OR (sql_mismatch AND is_new)`.** `is_new` alone is not a +defect. `sql_mismatch AND NOT is_new` is a pre-existing divergence that d516974 did not +cause. The script recomputes `is_defect` from each adjudicator's own returned booleans and +logs a warning when the adjudicator's prose disagrees with its own dimensions. + +Agents were also warned, in the shared prompt, against the specific failure that wrecked the +earlier audit: reaching for a standard citation to dignify a finding that is really about +internal coherence. Aggregates, operators, text search objects and indexes are outside the +standard's ken, so `sql_mismatch` is false for them by construction. + +## Attribution + +d516974 landed the same day as two siblings, `a9c350d` ("Don't try to re-order the +subcommands of CREATE SCHEMA") and `404db8f` ("Execute foreign key constraints in CREATE +SCHEMA at the end"), and was followed by `049b742` ("psql: Tighten heuristics for BEGIN/END +within CREATE SCHEMA"). a9c350d is the commit that made CREATE SCHEMA rely on a temporarily +prepended `search_path` instead of rewriting each element's schema name, so several +observations in this area belong to it, not to d516974. The `pre` build exists to settle +that question empirically, and the report keeps sibling-commit findings in a separate +section. + +## Method + +Nine phases, driven by one deterministic script (preserved at +`~/.claude/projects/.../workflows/scripts/create-schema-d516974-audit-wf_784c5f4f-e9b.js`): + +1. **Recon** (6 agents) -- commit forensics and the commit's own principles; the execution + environment at code level; the backend code newly reachable from inside CREATE SCHEMA; + the client-side code (psqlscan.l, tab completion, ecpg); the SQL standard's + `<schema definition>` rules; and existing test coverage. Each wrote a dossier that later + phases read instead of redoing the work. +2. **Hunt** (16 agents, one per lens) -- each lens generated at least twelve falsifiable + theories and then *ran* them on all three builds, reporting only observations it could + quote from a real transcript, plus a list of what it checked and found correct. +3. **Triage** (1 agent) -- dedupe into a numbered candidate list, spot-checking claims + against their evidence files. +4. **Replicate** -- an independent agent per candidate re-derived the observation from + scratch, ran it on `new`, `old` and `pre`, ran the equivalent command *outside* CREATE + SCHEMA as a control, minimized the reproduction, and named the mechanism with file:line. +5. **Adjudicate** -- one agent per replicated candidate applied the rubric. +6. **Refute** -- one adversarial agent per surviving defect, instructed to kill it and to + default to refuted when uncertain. +7. **Coherence** (2 agents) -- a shared-premise pass (when several findings rest on one + claim, adjudicate the claim once, adversarially) and a completeness critic whose answer + becomes the report's limitations section. +8. **Tests** (5 agents, serial) -- plan, write core regression tests, write client-side + tests, adversarially verify, polish. +9. **Report** (2 agents) -- write, then fact-check every quoted transcript, file:line + citation and standard citation by re-running and re-reading. + +Phases 4 to 6 run as one pipeline per candidate, so a candidate can be under refutation +while another is still being replicated. + +## Scheduling + +The owner asked that a budget stop cost 1-2 in-flight agents rather than 16. The first +implementation ran everything in fixed chunks of two, which he rejected: it would have left +the budget unused when the session window expired. His correction asked for an adaptive +scheme, and that is what shipped. + +Each phase runs on a worker pool that fills to 16 immediately -- every worker takes its first +item unconditionally -- and then gates only *replacements*. When a worker finishes an item +it launches another only if + +``` +budget.remaining() >= ((inFlight + 1) + reserve) * estCostPerAgent * 1.2 +``` + +otherwise it retires permanently, so concurrency tapers 16 -> ... -> 1 as the budget drains +and the run ends with one agent in flight instead of sixteen. `estCostPerAgent` is +`(budget.spent() - baselineAtStart) / liveCompletions`, with a 45k-output-token prior until +three live agents have finished; `budget.spent()` is a cumulative session-shared counter, so +it is baselined per invocation, and a completion counts as "live" only if its surrounding +delta exceeds 2k output tokens, so cached replays on resume do not drag the estimate to zero. +`reserve` holds back capacity for phases that still must run (8 agents through Hunt and the +candidate chains, 7 through Coherence) so the taper starts early enough that the report still +gets written. Retirement is logged, and a pool that stops with items unstarted logs +`POOL STOPPED EARLY: n/m`; resuming the run ID replays completed agents from the +content-addressed cache and runs only the remainder. + +**This run took the fallback branch the owner authorized.** `budget.remaining()` is finite +only when a token target is set for the turn; none was, so it was `Infinity`, the gate always +passed, and the run executed as a flat 16-wide pool. Measured 7 minutes into the Hunt phase: +24 agent transcripts, 15 running concurrently. The taper code is present and engages +whenever a target is set. + +## What the orchestrator did itself + +Outside the workflow, the orchestrating model: built the three trees and the harness; found +four seed observations by hand and put them in the shared prompt so agents would extend +rather than rediscover them; and independently checked two headline recon claims with +explicit outside-CREATE-SCHEMA controls, confirming one and refuting the other +(`csxrun/evidence/orch-verify1.txt`). It wrote this file. It did not write the report or +any test. + +## Outcome + +* **83 subagents**, 0 errors, 8,252,907 subagent output tokens, 2,299 tool calls, 2h59m + wall clock (workflow run `wf_784c5f4f-e9b`, three invocations -- see + [Interruptions](#interruptions)). +* 6 recon dossiers -> 16 hunt lenses -> **72 raw observations** against roughly 450 things + each lens checked and found correct -> **19 deduped candidates** -> 3 survived the + refute stage. +* The report presents **4 defects attributable to d516974** (D1-D4), a separate section of + findings belonging to the sibling commit `a9c350d`, a section on what was examined and + found correct, and the 16 rejected candidates with the reason each was rejected. +* The report writer reinstated narrowed forms of two candidates the refute stage had + killed (the `SET role` clause, D2, and tab completion, D4). It did so openly: D2 records + `is_new: true for this spelling, false for the rule` and carries an Attribution paragraph + saying the rule violation is reachable on `pre` by a harder route, and D4 carries a + "Counterarguments a reviewer should weigh" block. Both rest on `per_principles = true`, + which is sufficient under the rubric on its own. A reader who disagrees with that reading + should read those two blocks first. + +## Tests on this branch + +There is **no fix on this branch**, so the tests encode what a *correct* implementation +would print and therefore **fail on master by design**. The failure diff is the +demonstration. 339 lines were added across six test files; no non-test file was touched. + +Verified by the orchestrator, not by an agent: + +* With the additions: `regress/regress` (create_schema), `plpgsql/regress` (plpgsql_misc), + `psql/001_basic` (subtests 135-137) and `psql/010_tab_completion` (subtests 110-111) fail, + each only on the new material. Three further meson tests -- `pg_upgrade/002_pg_upgrade`, + `recovery/027_stream_regress`, `test_plan_advice/001_replan_regress` -- fail collaterally + because each runs the core `parallel_schedule`. +* Each added block ships its own control, and every control passes: `001_basic` subtests + 138-140 (the same input with a non-routine clause splits correctly) and + `010_tab_completion` subtest 112 (the same completion works at top level). +* With the six files stashed, all six suites are green: 243 + 13 + 134 + 101 + 2 + 5 + subtests pass, 0 failures. + +Reproduce: + +``` +export TMPDIR=/home/nm/src/pg/csxrun/tmp +meson test -C /home/nm/src/pg/csx-build --suite regress --suite plpgsql --suite psql \ + --num-processes 1 +``` + +## Interruptions + +The run was stopped and resumed twice. Completed agents replay from a content-addressed +cache keyed on (prompt, opts), so each stop cost only the agents in flight; of 25 starts +across the first two invocations only 2 keys were started twice. + +1. After the first two recon agents, to replace the rejected chunk-of-two scheduler with the + adaptive pool described above. +2. After the hunt phase, on noticing that 24 candidates times 3 chain agents would exhaust + the agent cap before the test and report phases could run. The fix raised the cap to the + owner's stated 90, capped triage at 20 candidates, and added a guard that stops the chain + phase from taking a new candidate once `SPAWNED + 3` would cross `90 - 9`, permanently + reserving 9 agents for coherence, tests and the report. + +## A host condition that contaminates some agent transcripts + +`/tmp` on this machine is a **3.9 GB tmpfs**, and it reached 100% full (27 MB free) partway +through the run, from unrelated work in other directories. `pgrun.sh` and `pgsh.sh` +originally created their throwaway clusters under `/tmp`, so during that window `initdb` +failed with + +``` +FATAL: could not write to file "base/4/2675": No space left on device +``` + +Any such line in an evidence transcript is that condition, **not** a PostgreSQL defect and +not a finding. The harness was repointed to `/home/nm/src/pg/csxrun/tmp` (on `/`, which had +22 GB free) and the affected checks were re-run. Nothing was deleted from `/tmp`. A +reviewer re-running the suites should set `TMPDIR` to a disk-backed directory, because +`pg_regress` puts its socket directory under `TMPDIR`. + +## What the orchestrator verified by hand + +Independently of the agents, and recorded in `csxrun/evidence/orch-verify*.txt`: + +* **D1 reproduced from scratch, with a control the agents had not run in that form.** On + `new`, `CREATE SCHEMA m CREATE FUNCTION f() ... GRANT ALL ON SCHEMA m TO begin;` followed + by `VACUUM;` yields `ERROR: VACUUM cannot run inside a transaction block` and the next + statement never runs, proving psql merged them into one message; on `old` the element is a + syntax error and splitting is correct; and replacing the routine clause with + `CREATE VIEW v AS SELECT 1` makes the identical GRANT split correctly. +* **`SET search_path FROM CURRENT` confirmed** to capture the internal temporary path + (`sp1, "$user", public`) where the session value is `"$user", public`. +* **One recon claim refuted**: that `CREATE SCHEMA` causes `pg_type.typdefault` to be stored + unqualified. A control -- create the schema and function first, `SET search_path = sm, + public`, then `CREATE DOMAIN sm.dm AS int DEFAULT dflt()` -- produces the identical split + outside `CREATE SCHEMA`, so it is pre-existing deparse behaviour. Three separate hunt + lenses had reported it; the pipeline dropped it. +* **The four highest-severity rejections and the two adjudications that bypassed the refute + stage were read and checked**, and their reasoning holds. +* **The test outcome above**, both with and without the additions. diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl index 028df33..189f82c 100644 --- a/src/bin/psql/t/001_basic.pl +++ b/src/bin/psql/t/001_basic.pl @@ -577,4 +577,44 @@ psql_fails_like( qr/wrong key/, '\unrestrict does not do backquote expansion'); +# +# AUDIT ADDITION: a user-visible defect of commit +# d516974840f4059d331ae6057ede3e4edd3c6747 ("Support more object types within +# CREATE SCHEMA."). The first case below records the behavior that a CORRECT +# implementation would produce, so it FAILS against master; the failure is +# the demonstration of the defect. The second case is a passing control. +# +# FINDING C03 (new in d516974). Correct behavior: psql ends a CREATE SCHEMA +# statement at its semicolon, so the VACUUM and the SELECT that follow it are +# separate queries and both run. +# Currently FAILS: the CREATE FUNCTION clause arms psqlscan.l's BEGIN/END +# counter, and psqlscan.l clears that per-clause state only on the token +# "create" -- GRANT is the one CREATE SCHEMA clause not starting with CREATE. +# So the view named "begin" in the GRANT clause is counted as the start of a +# routine body, and psql sends all three statements as one query: the VACUUM +# then fails inside the resulting implicit transaction block and takes the +# CREATE SCHEMA down with it. +psql_like( + $node, + qq{CREATE SCHEMA cs_gb + CREATE VIEW begin AS SELECT 1 AS one + CREATE FUNCTION cs_gb_f() RETURNS int LANGUAGE sql AS 'select 1' + GRANT SELECT ON begin TO public; +VACUUM; +SELECT 'split ok' AS result;}, + qr/^split ok$/m, + 'CREATE SCHEMA ends at its semicolon'); + +# Control: the same shape with no routine clause, which splits correctly. +psql_like( + $node, + qq{CREATE SCHEMA cs_gc + CREATE VIEW begin2 AS SELECT 1 AS one + CREATE VIEW cs_gc_v AS SELECT 1 + GRANT SELECT ON begin2 TO public; +VACUUM; +SELECT 'split ok' AS result;}, + qr/^split ok$/m, + 'control: CREATE SCHEMA with no routine clause'); + done_testing(); diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl index 64e27ef..099ac3d 100644 --- a/src/bin/psql/t/010_tab_completion.pl +++ b/src/bin/psql/t/010_tab_completion.pl @@ -126,6 +126,29 @@ sub clear_line return; } +# Like check_completion(), but report a test failure rather than dying when +# the expected completion does not happen. (check_completion() waits for its +# pattern and dies on timeout, which would abort the rest of this file.) The +# ^U and \r sent after the input also take the place of clear_query(). +sub check_completion_nofail +{ + my ($send, $pattern, $annotation) = @_; + + # report test failures from caller location + local $Test::Builder::Level = $Test::Builder::Level + 1; + + my $out = $h->query_until(qr/Query buffer reset.*postgres=# $/s, + $send . "\025\\r\n"); + my $okay = ($out =~ $pattern); + ok($okay, $annotation); + # for debugging, log actual output if it didn't match + local $Data::Dumper::Terse = 1; + local $Data::Dumper::Useqq = 1; + diag 'Actual output was ' . Dumper($out) . "Did not match \"$pattern\"\n" + if !$okay; + return; +} + # check basic command completion: SEL<tab> produces SELECT<space> check_completion("SEL\t", qr/SELECT /, "complete SEL<tab> to SELECT"); @@ -460,6 +483,78 @@ check_completion("FR\t", qr/FROM /, clear_query(); +# +# AUDIT ADDITION: a user-visible defect of commit +# d516974840f4059d331ae6057ede3e4edd3c6747 ("Support more object types within +# CREATE SCHEMA."). The two check_completion_nofail() cases below record the +# completions that a CORRECT implementation would offer, so they FAIL against +# master; the failures are the demonstration of the defect. The four +# check_completion() cases before them and the two after are passing controls. +# + +# Two clause types that d516974 did convert to TailMatches(), so that their +# completions fire inside CREATE SCHEMA. +check_completion( + "CREATE SCHEMA s CREATE DOMAIN d A\t", + qr/CREATE DOMAIN d AS /, + "complete CREATE DOMAIN inside CREATE SCHEMA"); + +clear_query(); + +check_completion( + "CREATE SCHEMA s CREATE TEXT SEARCH CONF\t", + qr/TEXT SEARCH CONFIGURATION /, + "complete CREATE TEXT SEARCH inside CREATE SCHEMA"); + +clear_query(); + +# d516974 also added COLLATION and TYPE to the clause list psql offers after +# CREATE SCHEMA ... CREATE, so psql agrees that both are legal there. +check_completion( + "CREATE SCHEMA s CREATE COLL\t", + qr/CREATE SCHEMA s CREATE COLLATION /, + "complete CREATE SCHEMA ... CREATE COLL<tab> to COLLATION"); + +clear_query(); + +check_completion( + "CREATE SCHEMA s CREATE TY\t", + qr/CREATE SCHEMA s CREATE TYPE /, + "complete CREATE SCHEMA ... CREATE TY<tab> to TYPE"); + +clear_query(); + +# FINDING O3 (new in d516974). Correct behavior: having offered COLLATION and +# TYPE as clauses just above, psql goes on completing them inside CREATE +# SCHEMA the way it does at top level (the two controls after these). +# Currently FAILS: those two rules still use anchored Matches()/HeadMatches(), +# which cannot match once "CREATE SCHEMA s" precedes them; d516974 converted +# only CREATE DOMAIN and CREATE TEXT SEARCH. +check_completion_nofail( + "CREATE SCHEMA s CREATE COLLATION c FR\t", + qr/CREATE COLLATION c FROM /, + "complete CREATE COLLATION inside CREATE SCHEMA"); + +check_completion_nofail( + "CREATE SCHEMA s CREATE TYPE t AS EN\t", + qr/CREATE TYPE t AS ENUM /, + "complete CREATE TYPE inside CREATE SCHEMA"); + +# Controls: the same two completions at top level, which do work. +check_completion( + "CREATE COLLATION c FR\t", + qr/CREATE COLLATION c FROM /, + "complete CREATE COLLATION at top level"); + +clear_query(); + +check_completion( + "CREATE TYPE t AS EN\t", + qr/CREATE TYPE t AS ENUM /, + "complete CREATE TYPE at top level"); + +clear_query(); + # send psql an explicit \q to shut it down, else pty won't close properly $h->quit or die "psql returned $?"; diff --git a/src/pl/plpgsql/src/expected/plpgsql_misc.out b/src/pl/plpgsql/src/expected/plpgsql_misc.out index ffb377f..87c4585 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_misc.out +++ b/src/pl/plpgsql/src/expected/plpgsql_misc.out @@ -29,6 +29,41 @@ CREATE OR REPLACE PROCEDURE public.test2(IN x integer) BEGIN ATOMIC SELECT (x + 2); END +-- +-- AUDIT ADDITION: a user-visible defect of commit +-- d516974840f4059d331ae6057ede3e4edd3c6747 ("Support more object types within +-- CREATE SCHEMA."). Unlike every case above, this one records the output +-- that a CORRECT implementation would produce, so it FAILS against master; +-- the failure diff is the demonstration of the defect. +-- +-- FINDING C14 (new in d516974). Correct behavior: a CREATE SCHEMA whose +-- element is a routine with a BEGIN ATOMIC body is writable as a direct +-- statement in PL/pgSQL, exactly like the same routine outside CREATE SCHEMA +-- above, and creates misc_schema.test3(). +-- Currently FAILS: make_execsql_stmt() counts BEGIN/END only after a leading +-- CREATE [OR REPLACE] {FUNCTION|PROCEDURE}, never after CREATE SCHEMA, so it +-- ends the statement at the first semicolon inside the routine body. psql's +-- copy of this heuristic learned about CREATE SCHEMA in d516974 and 049b742; +-- pl_gram.y's copy did not. +do +$$ + begin + create schema misc_schema + create function test3() returns int + begin atomic + select 3 + 3; + end; + end +$$; +\sf misc_schema.test3 +CREATE OR REPLACE FUNCTION misc_schema.test3() + RETURNS integer + LANGUAGE sql +BEGIN ATOMIC + SELECT (3 + 3); +END +drop schema misc_schema cascade; +NOTICE: drop cascades to function misc_schema.test3() -- Test %TYPE and %ROWTYPE error cases create table misc_table(f1 int); do $$ declare x foo%type; begin end $$; diff --git a/src/pl/plpgsql/src/sql/plpgsql_misc.sql b/src/pl/plpgsql/src/sql/plpgsql_misc.sql index 0bc39fc..d11f9a1 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_misc.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_misc.sql @@ -21,6 +21,37 @@ $$; \sf test1 \sf test2 +-- +-- AUDIT ADDITION: a user-visible defect of commit +-- d516974840f4059d331ae6057ede3e4edd3c6747 ("Support more object types within +-- CREATE SCHEMA."). Unlike every case above, this one records the output +-- that a CORRECT implementation would produce, so it FAILS against master; +-- the failure diff is the demonstration of the defect. +-- +-- FINDING C14 (new in d516974). Correct behavior: a CREATE SCHEMA whose +-- element is a routine with a BEGIN ATOMIC body is writable as a direct +-- statement in PL/pgSQL, exactly like the same routine outside CREATE SCHEMA +-- above, and creates misc_schema.test3(). +-- Currently FAILS: make_execsql_stmt() counts BEGIN/END only after a leading +-- CREATE [OR REPLACE] {FUNCTION|PROCEDURE}, never after CREATE SCHEMA, so it +-- ends the statement at the first semicolon inside the routine body. psql's +-- copy of this heuristic learned about CREATE SCHEMA in d516974 and 049b742; +-- pl_gram.y's copy did not. +do +$$ + begin + create schema misc_schema + create function test3() returns int + begin atomic + select 3 + 3; + end; + end +$$; + +\sf misc_schema.test3 + +drop schema misc_schema cascade; + -- Test %TYPE and %ROWTYPE error cases create table misc_table(f1 int); diff --git a/src/test/regress/expected/create_schema.out b/src/test/regress/expected/create_schema.out index b9ae4c4..344741a 100644 --- a/src/test/regress/expected/create_schema.out +++ b/src/test/regress/expected/create_schema.out @@ -322,5 +322,83 @@ drop cascades to type regress_schema_misc.cs_range drop cascades to function regress_schema_misc.cs_type_out(regress_schema_misc.cs_type) drop cascades to type regress_schema_misc.cs_type drop cascades to function regress_schema_misc.cs_type_in(cstring) +-- +-- AUDIT ADDITIONS: user-visible defects of commit +-- d516974840f4059d331ae6057ede3e4edd3c6747 ("Support more object types within +-- CREATE SCHEMA."). Unlike every case above, each case below records the +-- output that a CORRECT implementation would produce, so each one FAILS +-- against master; the failure diff is the demonstration of the defect. +-- +-- FINDING O1 (new in d516974). Correct behavior: a SET clause for "role" or +-- "session_authorization" in a CREATE FUNCTION element behaves just as it +-- does outside CREATE SCHEMA, so both functions below are created in the new +-- schema, owned by the AUTHORIZATION role, with the setting in proconfig. +-- Currently FAILS: CreateSchemaCommand runs the elements with +-- InLocalUserIdChange() true, so guc.c's GUC_NOT_WHILE_SEC_REST test rejects +-- the SET clause, naming a security-definer function that is not there. Two +-- call paths reach that test and both need dealing with: CreateFunction -> +-- GUCArrayAdd -> validate_option_array_item, which only validates +-- (changeVal = false) and is what fails today, and then ProcedureCreate -> +-- ProcessGUCArray, which applies proconfig around the language validator. A +-- fix must not simply let the value through: SetSessionAuthorization() +-- asserts that the security restriction context is clear. +CREATE SCHEMA regress_schema_setrole AUTHORIZATION regress_create_schema_role + CREATE FUNCTION cs_setrole() RETURNS int LANGUAGE sql + SET role = 'regress_create_schema_role' AS 'SELECT 1' + CREATE FUNCTION cs_setsess() RETURNS int LANGUAGE sql + SET session_authorization = 'regress_create_schema_role' AS 'SELECT 2'; +SELECT p.proname, pg_get_userbyid(p.proowner) AS owner, p.proconfig + FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = 'regress_schema_setrole' ORDER BY 1; + proname | owner | proconfig +------------+----------------------------+---------------------------------------------------- + cs_setrole | regress_create_schema_role | {role=regress_create_schema_role} + cs_setsess | regress_create_schema_role | {session_authorization=regress_create_schema_role} +(2 rows) + +SET client_min_messages = warning; +DROP SCHEMA regress_schema_setrole CASCADE; +RESET client_min_messages; +-- FINDING C08 (not new in d516974: the boundary is a9c350d, which made +-- CREATE SCHEMA hand the new schema to its subcommands as a prepended +-- search_path entry). Correct behavior: a schema named exactly "$user" +-- receives the objects created by its own subcommands, like any other schema. +-- Currently FAILS: "$user" in a search_path is a macro for the current role +-- name, so the elements land in whatever the rest of the path resolves to, +-- or, with an empty path, nowhere at all. Both halves run in an aborted +-- transaction, so neither the buggy nor the fixed behavior can leave objects +-- behind in another schema. +BEGIN; +CREATE SCHEMA "$user" + CREATE TABLE cs_dollar_tab (a int) + CREATE FUNCTION cs_dollar_func() RETURNS int LANGUAGE sql AS 'SELECT 1'; +SELECT relnamespace::regnamespace AS tab_schema FROM pg_class + WHERE relname = 'cs_dollar_tab'; + tab_schema +------------ + "$user" +(1 row) + +SELECT pronamespace::regnamespace AS func_schema FROM pg_proc + WHERE proname = 'cs_dollar_func'; + func_schema +------------- + "$user" +(1 row) + +ROLLBACK; +-- Again, with an empty search_path, where the statement today reports a +-- search_path error for the very schema it just created. +BEGIN; +SET LOCAL search_path = ''; +CREATE SCHEMA "$user" CREATE TABLE cs_dollar_tab2 (a int); +SELECT relnamespace::regnamespace AS tab2_schema FROM pg_class + WHERE relname = 'cs_dollar_tab2'; + tab2_schema +------------- + "$user" +(1 row) + +ROLLBACK; -- Clean up DROP ROLE regress_create_schema_role; diff --git a/src/test/regress/sql/create_schema.sql b/src/test/regress/sql/create_schema.sql index 526bb3c..cec7f77 100644 --- a/src/test/regress/sql/create_schema.sql +++ b/src/test/regress/sql/create_schema.sql @@ -177,5 +177,65 @@ CREATE SCHEMA regress_schema_misc DROP SCHEMA regress_schema_misc CASCADE; +-- +-- AUDIT ADDITIONS: user-visible defects of commit +-- d516974840f4059d331ae6057ede3e4edd3c6747 ("Support more object types within +-- CREATE SCHEMA."). Unlike every case above, each case below records the +-- output that a CORRECT implementation would produce, so each one FAILS +-- against master; the failure diff is the demonstration of the defect. +-- + +-- FINDING O1 (new in d516974). Correct behavior: a SET clause for "role" or +-- "session_authorization" in a CREATE FUNCTION element behaves just as it +-- does outside CREATE SCHEMA, so both functions below are created in the new +-- schema, owned by the AUTHORIZATION role, with the setting in proconfig. +-- Currently FAILS: CreateSchemaCommand runs the elements with +-- InLocalUserIdChange() true, so guc.c's GUC_NOT_WHILE_SEC_REST test rejects +-- the SET clause, naming a security-definer function that is not there. Two +-- call paths reach that test and both need dealing with: CreateFunction -> +-- GUCArrayAdd -> validate_option_array_item, which only validates +-- (changeVal = false) and is what fails today, and then ProcedureCreate -> +-- ProcessGUCArray, which applies proconfig around the language validator. A +-- fix must not simply let the value through: SetSessionAuthorization() +-- asserts that the security restriction context is clear. +CREATE SCHEMA regress_schema_setrole AUTHORIZATION regress_create_schema_role + CREATE FUNCTION cs_setrole() RETURNS int LANGUAGE sql + SET role = 'regress_create_schema_role' AS 'SELECT 1' + CREATE FUNCTION cs_setsess() RETURNS int LANGUAGE sql + SET session_authorization = 'regress_create_schema_role' AS 'SELECT 2'; +SELECT p.proname, pg_get_userbyid(p.proowner) AS owner, p.proconfig + FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = 'regress_schema_setrole' ORDER BY 1; +SET client_min_messages = warning; +DROP SCHEMA regress_schema_setrole CASCADE; +RESET client_min_messages; + +-- FINDING C08 (not new in d516974: the boundary is a9c350d, which made +-- CREATE SCHEMA hand the new schema to its subcommands as a prepended +-- search_path entry). Correct behavior: a schema named exactly "$user" +-- receives the objects created by its own subcommands, like any other schema. +-- Currently FAILS: "$user" in a search_path is a macro for the current role +-- name, so the elements land in whatever the rest of the path resolves to, +-- or, with an empty path, nowhere at all. Both halves run in an aborted +-- transaction, so neither the buggy nor the fixed behavior can leave objects +-- behind in another schema. +BEGIN; +CREATE SCHEMA "$user" + CREATE TABLE cs_dollar_tab (a int) + CREATE FUNCTION cs_dollar_func() RETURNS int LANGUAGE sql AS 'SELECT 1'; +SELECT relnamespace::regnamespace AS tab_schema FROM pg_class + WHERE relname = 'cs_dollar_tab'; +SELECT pronamespace::regnamespace AS func_schema FROM pg_proc + WHERE proname = 'cs_dollar_func'; +ROLLBACK; +-- Again, with an empty search_path, where the statement today reports a +-- search_path error for the very schema it just created. +BEGIN; +SET LOCAL search_path = ''; +CREATE SCHEMA "$user" CREATE TABLE cs_dollar_tab2 (a int); +SELECT relnamespace::regnamespace AS tab2_schema FROM pg_class + WHERE relname = 'cs_dollar_tab2'; +ROLLBACK; + -- Clean up DROP ROLE regress_create_schema_role;
