Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
409 changes: 409 additions & 0 deletions SCENARIOS-pg-loader-compat.md

Large diffs are not rendered by default.

102 changes: 102 additions & 0 deletions docs/plans/2026-05-11-pg-loader-compat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# PG Loader Compatibility Implementation Plan

> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.

**Goal:** Close PostgreSQL-supported syntax and analysis gaps that currently fail in omni's PG parser/catalog loader.

**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.

**Tech Stack:** Go, `go test`, omni `pg/parser`, omni `pg/catalog`.

### Task 1: Zero-column table and `MATCH SIMPLE`

**Files:**
- Modify: `pg/catalog/tablecmds.go`
- Modify: `pg/parser/create_table.go`
- Test: `pg/catalog/loader_compat_test.go`

**Step 1: Write the failing tests**

Add tests that call `LoadSQL` for `CREATE TABLE t ();` and a foreign key using `MATCH SIMPLE`.

**Step 2: Run tests to verify they fail**

Run: `go test ./pg/catalog -run 'TestLoaderCompat' -count=1`
Expected: zero-column table fails with `tables must have at least one column`; `MATCH SIMPLE` fails near `SIMPLE`.

**Step 3: Implement minimal fixes**

Remove the catalog rejection for zero-column regular tables. Update `parseKeyMatch` to consume the `SIMPLE` token as well as identifier text.

**Step 4: Run tests to verify they pass**

Run: `go test ./pg/catalog -run 'TestLoaderCompat' -count=1`
Expected: PASS.

### Task 2: Function comments and `RETURNS TABLE`

**Files:**
- Modify: `pg/parser/define.go` or relevant function argument parser
- Modify: `pg/catalog/functioncmds.go`
- Test: `pg/catalog/loader_compat_test.go`

**Step 1: Write failing tests**

Add tests for `COMMENT ON FUNCTION f(arg_name integer)` and `RETURNS TABLE(...) LANGUAGE plpgsql`.

**Step 2: Verify red**

Run targeted `go test` and confirm failures are from named argument parsing or return validation.

**Step 3: Implement minimal fixes**

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.

**Step 4: Verify green**

Run the targeted loader compatibility test.

### Task 3: View analyzer expression gaps

**Files:**
- Modify: `pg/catalog/analyze.go`
- Test: `pg/catalog/loader_compat_test.go`

**Step 1: Write failing tests**

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.

**Step 2: Verify red**

Run targeted tests and record exact failing analyzer path for each case.

**Step 3: Implement one analyzer fix per failure**

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.

**Step 4: Verify green**

Run targeted tests, then relevant analyzer/view regression tests.

### Task 4: Index partition attach and FK type compatibility

**Files:**
- Modify: `pg/catalog/alter.go`
- Modify: `pg/catalog/constraint.go`
- Test: `pg/catalog/loader_compat_test.go`

**Step 1: Write failing tests**

Add tests for `ALTER INDEX parent ATTACH PARTITION child` and bigint FK referencing integer PK.

**Step 2: Verify red**

Run targeted tests and confirm whether failure is relation lookup, index parent metadata, or type compatibility direction.

**Step 3: Implement minimal fixes**

Fix partitioned parent index lookup/metadata and align FK compatibility with PostgreSQL's accepted implicit coercion direction.

**Step 4: Verify green**

Run targeted tests and partition/FK regression tests.
85 changes: 85 additions & 0 deletions pg/catalog/alter.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ func (c *Catalog) AlterTableStmt(stmt *nodes.AlterTableStmt) error {
relName = stmt.Relation.Relname
}

if nodes.ObjectType(stmt.ObjType) == nodes.OBJECT_INDEX {
return c.AlterIndexStmt(stmt)
}
if nodes.ObjectType(stmt.ObjType) == nodes.OBJECT_SEQUENCE {
_, err := c.findSequence(schemaName, relName)
return err
}

schema, rel, err := c.findRelation(schemaName, relName)
if err != nil {
return err
Expand Down Expand Up @@ -243,6 +251,73 @@ func (c *Catalog) AlterTableStmt(stmt *nodes.AlterTableStmt) error {
return nil
}

// AlterIndexStmt applies ALTER INDEX commands.
//
// pg: src/backend/commands/tablecmds.c — AlterTable dispatch for OBJECT_INDEX
func (c *Catalog) AlterIndexStmt(stmt *nodes.AlterTableStmt) error {
if stmt.Relation == nil {
return errInvalidParameterValue("ALTER INDEX requires an index name")
}
schema, err := c.resolveTargetSchema(stmt.Relation.Schemaname)
if err != nil {
return err
}
parentIdx := schema.Indexes[stmt.Relation.Relname]
if parentIdx == nil {
return errUndefinedObject("index", stmt.Relation.Relname)
}
if stmt.Cmds == nil {
return nil
}

for _, item := range stmt.Cmds.Items {
atc, ok := item.(*nodes.AlterTableCmd)
if !ok {
continue
}
switch nodes.AlterTableType(atc.Subtype) {
case nodes.AT_AttachPartition:
pc, ok := atc.Def.(*nodes.PartitionCmd)
if !ok {
return fmt.Errorf("AT_AttachPartition: expected PartitionCmd")
}
if err := c.atExecAttachIndexPartition(parentIdx, pc); err != nil {
return err
}
}
}
return nil
}

func (c *Catalog) atExecAttachIndexPartition(parentIdx *Index, pc *nodes.PartitionCmd) error {
if pc == nil || pc.Name == nil {
return errInvalidParameterValue("ATTACH PARTITION requires an index name")
}
parentRel := c.findRelByOID(parentIdx.RelOID)
if parentRel == nil || parentRel.RelKind != 'p' {
return errInvalidObjectDefinition(fmt.Sprintf("index %q is not on a partitioned table", parentIdx.Name))
}

childSchema, err := c.resolveTargetSchema(pc.Name.Schemaname)
if err != nil {
return err
}
childIdx := childSchema.Indexes[pc.Name.Relname]
if childIdx == nil {
return errUndefinedObject("index", pc.Name.Relname)
}
childRel := c.findRelByOID(childIdx.RelOID)
if childRel == nil {
return errUndefinedTable(pc.Name.Relname)
}
if childRel.PartitionOf != parentRel.OID {
return errInvalidObjectDefinition(fmt.Sprintf("index %q is not on a partition of %q", childIdx.Name, parentRel.Name))
}

c.recordDependency('i', childIdx.OID, 0, 'i', parentIdx.OID, 0, DepInternal)
return nil
}

// execAlterTableCmd executes a single ALTER TABLE subcommand.
func (c *Catalog) execAlterTableCmd(schema *Schema, rel *Relation, relName string, atc *nodes.AlterTableCmd, identityOptions *nodes.List, recurse, recursing bool) error {
cascade := atc.Behavior == int(nodes.DROP_CASCADE)
Expand Down Expand Up @@ -274,6 +349,8 @@ func (c *Catalog) execAlterTableCmd(schema *Schema, rel *Relation, relName strin
analyzed = coerced
}
col.DefaultAnalyzed = analyzed
c.recordDependencyOnSingleRelExprForObject('r', rel.OID, int32(col.AttNum), analyzed, rel.OID,
DepNormal, DepNormal)
rte := c.buildRelationRTE(rel)
col.Default = c.DeparseExpr(analyzed, []*RangeTableEntry{rte}, false)
}
Expand All @@ -285,6 +362,8 @@ func (c *Catalog) execAlterTableCmd(schema *Schema, rel *Relation, relName strin
analyzed = coerced
}
col.DefaultAnalyzed = analyzed
c.recordDependencyOnSingleRelExprForObject('r', rel.OID, int32(col.AttNum), analyzed, rel.OID,
DepNormal, DepNormal)
rte := c.buildRelationRTE(rel)
genExpr := c.DeparseExpr(analyzed, []*RangeTableEntry{rte}, false)
col.GenerationExpr = genExpr
Expand Down Expand Up @@ -320,6 +399,8 @@ func (c *Catalog) execAlterTableCmd(schema *Schema, rel *Relation, relName strin
analyzed = coerced
}
rel.Columns[idx].DefaultAnalyzed = analyzed
c.recordDependencyOnSingleRelExprForObject('r', rel.OID, int32(rel.Columns[idx].AttNum), analyzed, rel.OID,
DepNormal, DepNormal)
}
rte := c.buildRelationRTE(rel)
defStr := c.DeparseExpr(analyzed, []*RangeTableEntry{rte}, false)
Expand Down Expand Up @@ -1891,6 +1972,8 @@ func extractObjectName(obj nodes.Node) (schema, name string) {
switch n := obj.(type) {
case *nodes.List:
return qualifiedName(n)
case *nodes.ObjectWithArgs:
return qualifiedName(n.Objname)
case *nodes.String:
return "", n.Str
default:
Expand Down Expand Up @@ -2194,6 +2277,8 @@ func (c *Catalog) atSetExpression(rel *Relation, colName string, expr nodes.Node
analyzed = coerced
}
col.DefaultAnalyzed = analyzed
c.recordDependencyOnSingleRelExprForObject('r', rel.OID, int32(col.AttNum), analyzed, rel.OID,
DepNormal, DepNormal)
rte := c.buildRelationRTE(rel)
col.GenerationExpr = c.DeparseExpr(analyzed, []*RangeTableEntry{rte}, false)
col.Default = col.GenerationExpr
Expand Down
Loading
Loading