Skip to content

Commit 84e155a

Browse files
authored
fix(pg/parser): KB-1/2/3 closure + Class A systematic silent-accept cleanup (#107)
Stacked on #105 #106. Closes oracle-discovered KB-1/KB-2/KB-3, audits + fixes 27+ Class A silent-accept sites, lands G1-GATE lint enforcement. See PR for full description. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent 774fc63 commit 84e155a

39 files changed

Lines changed: 2319 additions & 209 deletions

pg/parser/PAREN_AUDIT_DRIFT_BASELINE.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ publication.go parseRuleActionMulti
4646
schema.go parseAggregateWithArgtypesForComment
4747
schema.go parseFunctionWithArgtypesForComment
4848
schema.go parseOperatorWithArgtypesForComment
49+
schema.go parseSchemaStmtCreateOptTemp
4950
select.go parseFuncAliasClause
5051
select.go parseOptAliasClause
5152
select.go parseRelationOrFuncTable

pg/parser/PAREN_FOLLOWUPS.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ The following are post-merge items surfaced by Codex review at each phase. None
88

99
Discovered by Phase 2/3 oracle + fuzz. See `PAREN_KNOWN_BUGS.md` for full entries.
1010

11-
- **PAREN-KB-1**`(T JOIN U)` accepted without ON/USING. Fix in `parseJoinedTable` / `parseJoinQual`. Pinned by §2.7 `t.Skip` + fuzz known-mismatches.
12-
- **PAREN-KB-2**`Parse` returns only first RawStmt for multi-statement input like `SELECT * FROM (SELECT 1) SELECT 1`. Fix in `parser.go:Parse`. Top-level statement-list issue, out of `parenBeginsSubquery` scope.
13-
- **PAREN-KB-3**`LATERAL ()` accepted with empty body; PG rejects. Fix in `parseSelectWithParens` nil-body check. Pinned by §3.2 `t.Skip`.
11+
- **PAREN-KB-1**~~`(T JOIN U)` accepted without ON/USING~~ **CLOSED 2026-04-22** (commit a593131) via `parseJoinQual` strictness fix. Unskipped §2.7 oracle test; fuzz known-mismatches allowlist now empty.
12+
- **PAREN-KB-3**~~`LATERAL ()` accepted with empty body~~ **CLOSED 2026-04-22** (commit 3eed1fe) via `parseLateralTableRef` nil-body guard. Unskipped §3.2 oracle test.
13+
- **PAREN-KB-2**~~`Parse` accepts multi-statement input without `;` separator~~ **CLOSED 2026-04-22** via 4 upstream-blocker fixes (KB-2a/b/c/d) + parser.go:Parse needSeparator check reland. See PAREN_KNOWN_BUGS.md for the full commit chain.
1414

1515
## CI hardening follow-ups
1616

pg/parser/PAREN_KNOWN_BUGS.md

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,72 @@ so the signal isn't lost when future work reads those files.
7777
`TestParenOracleLateral/invalid_shapes_rejected/LATERAL_empty_parens`
7878
(skipped with a pointer to PAREN-KB-3).
7979

80-
## PAREN-KB-2 — `Parse()` silently drops trailing statements
80+
## PAREN-KB-2 — `Parse()` accepts statements without `;` separator — **CLOSED 2026-04-22**
81+
82+
**Status:** All 13 upstream blockers fixed (KB-2a/b/c/d), then `parser.go:Parse` `needSeparator` check re-applied. pgregress + oracle suites fully green. The KB-2 entry below documents the history of the attempt, the blockers surfaced, and the fix commits that unblocked it.
83+
84+
Closed by commit chain:
85+
- 0f5b7d2 KB-2a: parseRuleActionStmt NotifyStmt case
86+
- 6075da0 KB-2b: ALTER SEQUENCE SET LOGGED/UNLOGGED
87+
- 1808371 KB-2c: CREATE SCHEMA inline schema_element list
88+
- d7e9ba3 KB-2d: CREATE TEMP VIEW inside SCHEMA + DROP FUNCTION empty name
89+
- <this commit> KB-2 reland: needSeparator check in parser.go Parse loop
90+
91+
## PAREN-KB-2 — `Parse()` accepts statements without `;` separator — **history**
92+
93+
**Status:** fix attempted 2026-04-22 and reverted. Enforcing "cur must be `;` or EOF after each parseStmt" at parser.go:Parse surfaced 13 new pgregress failures — all pre-existing omni parser gaps that had been **masked** by the silent-accept behavior (CREATE RULE `DO INSTEAD NOTIFY x` body, SET inside transaction / savepoint blocks, CREATE-chained DDL). Before the statement-list strictness can land, those 13 upstream grammar gaps need to be fixed so the corresponding single-statement parses emit the right AST instead of truncating and letting a second parseStmt pick up the tail.
94+
95+
### A1 KB-2 full-surface survey results (2026-04-22 re-run)
96+
97+
Captured via clean baseline + in-place patch apply + clean revert, across all omni test surfaces:
98+
99+
- `go test ./pg/... -count=1`: 13 NEW FAILURE entries (all in pg/pgregress).
100+
- `go test -tags=oracle ./pg/parser/... -count=1`: 0 additional NEW FAILURE entries (full 188-probe oracle corpus still green under the patch).
101+
- Other packages (pg, pg/ast, pg/catalog, pg/completion, pg/parsertest, pg/plpgsql/parser): all still green.
102+
103+
**Total unique blockers: 13.** No new classes beyond what the delta reveals — oracle fence ran clean even under the stricter Parse loop.
104+
105+
Concrete blocker list, grouped by underlying parser gap (earlier draft mis-attributed some files; this is the accurate breakdown from the 2026-04-22 re-run):
106+
107+
**Category KB-2a — CREATE RULE + NotifyStmt action (4 failures, fix site: `publication.go:689 parseRuleActionStmt`)**
108+
- `copydml.sql:69` stmt[48]`create rule qqq as on insert to copydml_test do instead notify copydml_test`
109+
- `rules.sql:1017` stmt[474]`create rule r4 as on delete to rules_src do notify rules_src_deletion`
110+
- `with.sql:1721` stmt[291]`CREATE OR REPLACE RULE y_rule AS ON INSERT TO y DO INSTEAD NOTIFY foo`
111+
- `with.sql:1726` stmt[293]`CREATE OR REPLACE RULE y_rule AS ON INSERT TO y DO ALSO NOTIFY foo`
112+
113+
Root cause: `parseRuleActionStmt` enumerates 4 of 5 PG `RuleActionStmt` alternatives (SELECT/INSERT/UPDATE/DELETE) — NotifyStmt falls through `default: return nil, nil`. The 5th production is literally named in the function's own doc comment but not handled.
114+
115+
**Category KB-2b — ALTER SEQUENCE SET LOGGED/UNLOGGED (4 failures, fix site: ALTER SEQUENCE action parser)**
116+
- `identity.sql:535` stmt[266]`ALTER SEQUENCE identity_dump_logged_a_seq SET UNLOGGED`
117+
- `identity.sql:537` stmt[268]`ALTER SEQUENCE identity_dump_unlogged_a_seq SET LOGGED`
118+
- `sequence.sql:277` stmt[154]`ALTER SEQUENCE sequence_test_unlogged SET LOGGED`
119+
- `sequence.sql:279` stmt[155]`ALTER SEQUENCE sequence_test_unlogged SET UNLOGGED`
120+
121+
Root cause: PG 15+ added `SET LOGGED` / `SET UNLOGGED` to the ALTER SEQUENCE action list. omni's ALTER SEQUENCE action dispatcher stops at an earlier match, leaves `SET` / `UNLOGGED` / `LOGGED` as residual tail.
122+
123+
**Category KB-2c — CREATE SCHEMA schema_element inline body (3 failures, fix site: `parseCreateSchemaStmt`)**
124+
- `create_schema.sql:20` stmt[5]`CREATE SCHEMA AUTHORIZATION regress_create_schema_role` + following inline `CREATE TABLE ...` etc.
125+
- `create_schema.sql:33` stmt[11]`CREATE SCHEMA AUTHORIZATION CURRENT_ROLE` + schema_element list
126+
- `create_schema.sql:45` stmt[16]`CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE` + schema_element list
127+
128+
Root cause: PG CREATE SCHEMA grammar supports `CREATE SCHEMA [name] [AUTHORIZATION role] [schema_element [...]]` where schema_element is CREATE TABLE / CREATE VIEW / CREATE INDEX / etc. inline. omni's parser stops after AUTHORIZATION, leaves subsequent CREATE tokens as "next statement".
129+
130+
**Category KB-2d — fringe / TBD (2 failures, need spot-check):**
131+
- `errors.sql:139` stmt[38] — trailing "CREATE" after some preceding stmt in the DROP AGGREGATE test context. SQL shown by regress harness is `-- should fail` (comment) but the real content is the preceding DROP AGGREGATE stmt whose body isn't fully consumed.
132+
- `create_view.sql:162` stmt[36] — trailing `(` inside the "-- subqueries" CREATE VIEW test block. Likely a CREATE VIEW parser gap when the view body has subqueries.
133+
134+
Root cause: unknown without reading the extracted SQL precisely; spot-check at fix time.
135+
136+
**Recommended path:**
137+
1. Open follow-up issues for each of the 3 pattern classes (CREATE RULE action body, transaction-block SET, CREATE TRIGGER body).
138+
2. Fix each so the single statement consumes all its tokens.
139+
3. Then re-apply the KB-2 fix — `needSeparator` flag in parser.go's Parse loop requiring `;` or EOF between parseStmt calls. The fix itself is ~5 lines and already designed in commit history (see reverted diff at pg/parser/parser.go Parse loop on 2026-04-22).
140+
141+
Keep the oracle signal:
142+
- `pg/parser/testdata/paren-fuzz-corpus/seed-cases.txt` — NOT pinned (this is a parser.go-Parse bug, not a `(` dispatch bug).
143+
- `pg/parser/testdata/paren-fuzz-corpus/known-mismatches.txt` — NOT pinned; fuzz generator's `genReservedMisuse` no longer emits trailing-SELECT patterns.
144+
145+
## PAREN-KB-2 — `Parse()` silently drops trailing statements (original entry, kept for history)
81146

82147
- **Summary:** omni's top-level `Parse(sql string)` accepts inputs
83148
like `FROM (SELECT 1) SELECT 1` by returning only the first

0 commit comments

Comments
 (0)