Skip to content

Commit 58682c0

Browse files
rebelicerebelice
authored
Improve PostgreSQL loader compatibility coverage (#131)
Improve PG loader compatibility coverage
1 parent 8a05fdd commit 58682c0

24 files changed

Lines changed: 3380 additions & 154 deletions

‎SCENARIOS-pg-loader-compat.md‎

Lines changed: 409 additions & 0 deletions
Large diffs are not rendered by default.

‎docs/plans/2026-05-11-pg-loader-compat.md‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# PG Loader Compatibility Implementation Plan
2+
3+
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
4+
5+
**Goal:** Close PostgreSQL-supported syntax and analysis gaps that currently fail in omni's PG parser/catalog loader.
6+
7+
**Architecture:** Add focused regression tests in `pg/catalog` or `pg/parser` for each PG-supported construct, verify they fail for the expected reason, then make the narrowest parser/catalog/analyzer change for each root cause. Keep each change scoped to the component that rejects behavior PostgreSQL accepts.
8+
9+
**Tech Stack:** Go, `go test`, omni `pg/parser`, omni `pg/catalog`.
10+
11+
### Task 1: Zero-column table and `MATCH SIMPLE`
12+
13+
**Files:**
14+
- Modify: `pg/catalog/tablecmds.go`
15+
- Modify: `pg/parser/create_table.go`
16+
- Test: `pg/catalog/loader_compat_test.go`
17+
18+
**Step 1: Write the failing tests**
19+
20+
Add tests that call `LoadSQL` for `CREATE TABLE t ();` and a foreign key using `MATCH SIMPLE`.
21+
22+
**Step 2: Run tests to verify they fail**
23+
24+
Run: `go test ./pg/catalog -run 'TestLoaderCompat' -count=1`
25+
Expected: zero-column table fails with `tables must have at least one column`; `MATCH SIMPLE` fails near `SIMPLE`.
26+
27+
**Step 3: Implement minimal fixes**
28+
29+
Remove the catalog rejection for zero-column regular tables. Update `parseKeyMatch` to consume the `SIMPLE` token as well as identifier text.
30+
31+
**Step 4: Run tests to verify they pass**
32+
33+
Run: `go test ./pg/catalog -run 'TestLoaderCompat' -count=1`
34+
Expected: PASS.
35+
36+
### Task 2: Function comments and `RETURNS TABLE`
37+
38+
**Files:**
39+
- Modify: `pg/parser/define.go` or relevant function argument parser
40+
- Modify: `pg/catalog/functioncmds.go`
41+
- Test: `pg/catalog/loader_compat_test.go`
42+
43+
**Step 1: Write failing tests**
44+
45+
Add tests for `COMMENT ON FUNCTION f(arg_name integer)` and `RETURNS TABLE(...) LANGUAGE plpgsql`.
46+
47+
**Step 2: Verify red**
48+
49+
Run targeted `go test` and confirm failures are from named argument parsing or return validation.
50+
51+
**Step 3: Implement minimal fixes**
52+
53+
Accept optional argument names in `ObjectWithArgs` type lists and keep locating comments by argument type OIDs. Align `RETURNS TABLE` return validation with PostgreSQL's OUT parameter behavior.
54+
55+
**Step 4: Verify green**
56+
57+
Run the targeted loader compatibility test.
58+
59+
### Task 3: View analyzer expression gaps
60+
61+
**Files:**
62+
- Modify: `pg/catalog/analyze.go`
63+
- Test: `pg/catalog/loader_compat_test.go`
64+
65+
**Step 1: Write failing tests**
66+
67+
Add view tests for `concat_ws(text, variadic any)`, `jsonb ->> c.col_name` with `unnest(text[])` alias, CTE range resolution, and later LATERAL alias scope.
68+
69+
**Step 2: Verify red**
70+
71+
Run targeted tests and record exact failing analyzer path for each case.
72+
73+
**Step 3: Implement one analyzer fix per failure**
74+
75+
Use PostgreSQL-compatible type inference and range-table scope handling. Keep changes narrow and do not add broad fallback typing unless the test proves the exact missing behavior.
76+
77+
**Step 4: Verify green**
78+
79+
Run targeted tests, then relevant analyzer/view regression tests.
80+
81+
### Task 4: Index partition attach and FK type compatibility
82+
83+
**Files:**
84+
- Modify: `pg/catalog/alter.go`
85+
- Modify: `pg/catalog/constraint.go`
86+
- Test: `pg/catalog/loader_compat_test.go`
87+
88+
**Step 1: Write failing tests**
89+
90+
Add tests for `ALTER INDEX parent ATTACH PARTITION child` and bigint FK referencing integer PK.
91+
92+
**Step 2: Verify red**
93+
94+
Run targeted tests and confirm whether failure is relation lookup, index parent metadata, or type compatibility direction.
95+
96+
**Step 3: Implement minimal fixes**
97+
98+
Fix partitioned parent index lookup/metadata and align FK compatibility with PostgreSQL's accepted implicit coercion direction.
99+
100+
**Step 4: Verify green**
101+
102+
Run targeted tests and partition/FK regression tests.

‎pg/catalog/alter.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,14 @@ func (c *Catalog) AlterTableStmt(stmt *nodes.AlterTableStmt) error {
139139
relName = stmt.Relation.Relname
140140
}
141141

142+
if nodes.ObjectType(stmt.ObjType) == nodes.OBJECT_INDEX {
143+
return c.AlterIndexStmt(stmt)
144+
}
145+
if nodes.ObjectType(stmt.ObjType) == nodes.OBJECT_SEQUENCE {
146+
_, err := c.findSequence(schemaName, relName)
147+
return err
148+
}
149+
142150
schema, rel, err := c.findRelation(schemaName, relName)
143151
if err != nil {
144152
return err
@@ -243,6 +251,73 @@ func (c *Catalog) AlterTableStmt(stmt *nodes.AlterTableStmt) error {
243251
return nil
244252
}
245253

254+
// AlterIndexStmt applies ALTER INDEX commands.
255+
//
256+
// pg: src/backend/commands/tablecmds.c — AlterTable dispatch for OBJECT_INDEX
257+
func (c *Catalog) AlterIndexStmt(stmt *nodes.AlterTableStmt) error {
258+
if stmt.Relation == nil {
259+
return errInvalidParameterValue("ALTER INDEX requires an index name")
260+
}
261+
schema, err := c.resolveTargetSchema(stmt.Relation.Schemaname)
262+
if err != nil {
263+
return err
264+
}
265+
parentIdx := schema.Indexes[stmt.Relation.Relname]
266+
if parentIdx == nil {
267+
return errUndefinedObject("index", stmt.Relation.Relname)
268+
}
269+
if stmt.Cmds == nil {
270+
return nil
271+
}
272+
273+
for _, item := range stmt.Cmds.Items {
274+
atc, ok := item.(*nodes.AlterTableCmd)
275+
if !ok {
276+
continue
277+
}
278+
switch nodes.AlterTableType(atc.Subtype) {
279+
case nodes.AT_AttachPartition:
280+
pc, ok := atc.Def.(*nodes.PartitionCmd)
281+
if !ok {
282+
return fmt.Errorf("AT_AttachPartition: expected PartitionCmd")
283+
}
284+
if err := c.atExecAttachIndexPartition(parentIdx, pc); err != nil {
285+
return err
286+
}
287+
}
288+
}
289+
return nil
290+
}
291+
292+
func (c *Catalog) atExecAttachIndexPartition(parentIdx *Index, pc *nodes.PartitionCmd) error {
293+
if pc == nil || pc.Name == nil {
294+
return errInvalidParameterValue("ATTACH PARTITION requires an index name")
295+
}
296+
parentRel := c.findRelByOID(parentIdx.RelOID)
297+
if parentRel == nil || parentRel.RelKind != 'p' {
298+
return errInvalidObjectDefinition(fmt.Sprintf("index %q is not on a partitioned table", parentIdx.Name))
299+
}
300+
301+
childSchema, err := c.resolveTargetSchema(pc.Name.Schemaname)
302+
if err != nil {
303+
return err
304+
}
305+
childIdx := childSchema.Indexes[pc.Name.Relname]
306+
if childIdx == nil {
307+
return errUndefinedObject("index", pc.Name.Relname)
308+
}
309+
childRel := c.findRelByOID(childIdx.RelOID)
310+
if childRel == nil {
311+
return errUndefinedTable(pc.Name.Relname)
312+
}
313+
if childRel.PartitionOf != parentRel.OID {
314+
return errInvalidObjectDefinition(fmt.Sprintf("index %q is not on a partition of %q", childIdx.Name, parentRel.Name))
315+
}
316+
317+
c.recordDependency('i', childIdx.OID, 0, 'i', parentIdx.OID, 0, DepInternal)
318+
return nil
319+
}
320+
246321
// execAlterTableCmd executes a single ALTER TABLE subcommand.
247322
func (c *Catalog) execAlterTableCmd(schema *Schema, rel *Relation, relName string, atc *nodes.AlterTableCmd, identityOptions *nodes.List, recurse, recursing bool) error {
248323
cascade := atc.Behavior == int(nodes.DROP_CASCADE)
@@ -274,6 +349,8 @@ func (c *Catalog) execAlterTableCmd(schema *Schema, rel *Relation, relName strin
274349
analyzed = coerced
275350
}
276351
col.DefaultAnalyzed = analyzed
352+
c.recordDependencyOnSingleRelExprForObject('r', rel.OID, int32(col.AttNum), analyzed, rel.OID,
353+
DepNormal, DepNormal)
277354
rte := c.buildRelationRTE(rel)
278355
col.Default = c.DeparseExpr(analyzed, []*RangeTableEntry{rte}, false)
279356
}
@@ -285,6 +362,8 @@ func (c *Catalog) execAlterTableCmd(schema *Schema, rel *Relation, relName strin
285362
analyzed = coerced
286363
}
287364
col.DefaultAnalyzed = analyzed
365+
c.recordDependencyOnSingleRelExprForObject('r', rel.OID, int32(col.AttNum), analyzed, rel.OID,
366+
DepNormal, DepNormal)
288367
rte := c.buildRelationRTE(rel)
289368
genExpr := c.DeparseExpr(analyzed, []*RangeTableEntry{rte}, false)
290369
col.GenerationExpr = genExpr
@@ -320,6 +399,8 @@ func (c *Catalog) execAlterTableCmd(schema *Schema, rel *Relation, relName strin
320399
analyzed = coerced
321400
}
322401
rel.Columns[idx].DefaultAnalyzed = analyzed
402+
c.recordDependencyOnSingleRelExprForObject('r', rel.OID, int32(rel.Columns[idx].AttNum), analyzed, rel.OID,
403+
DepNormal, DepNormal)
323404
}
324405
rte := c.buildRelationRTE(rel)
325406
defStr := c.DeparseExpr(analyzed, []*RangeTableEntry{rte}, false)
@@ -1891,6 +1972,8 @@ func extractObjectName(obj nodes.Node) (schema, name string) {
18911972
switch n := obj.(type) {
18921973
case *nodes.List:
18931974
return qualifiedName(n)
1975+
case *nodes.ObjectWithArgs:
1976+
return qualifiedName(n.Objname)
18941977
case *nodes.String:
18951978
return "", n.Str
18961979
default:
@@ -2194,6 +2277,8 @@ func (c *Catalog) atSetExpression(rel *Relation, colName string, expr nodes.Node
21942277
analyzed = coerced
21952278
}
21962279
col.DefaultAnalyzed = analyzed
2280+
c.recordDependencyOnSingleRelExprForObject('r', rel.OID, int32(col.AttNum), analyzed, rel.OID,
2281+
DepNormal, DepNormal)
21972282
rte := c.buildRelationRTE(rel)
21982283
col.GenerationExpr = c.DeparseExpr(analyzed, []*RangeTableEntry{rte}, false)
21992284
col.Default = col.GenerationExpr

0 commit comments

Comments
 (0)