Skip to content

fix(transform): single routing pass via claims; namespace-aware handlers; statement spans on facts - #323

Merged
pyramation merged 1 commit into
mainfrom
feat/ast-single-routing-pass
Jul 30, 2026
Merged

fix(transform): single routing pass via claims; namespace-aware handlers; statement spans on facts#323
pyramation merged 1 commit into
mainfrom
feat/ast-single-routing-pass

Conversation

@pyramation

Copy link
Copy Markdown
Collaborator

Summary

Stage 1 of the AST convergence plan (constructive-planning #1323). Three tightly-coupled changes to @pgsql/transform:

1. Fixes a live double-routing bug. Every "the walker does NOT auto-recurse into X" comment in transform.ts is false — @pgsql/traverse tag-synthesizes concrete typed fields (CreateStmt.relation, Constraint.pktable, TypeCast.typeName, CallStmt.funccall, …), verified empirically for all ~24 claimed cases. So each site was routed twice: once by the statement handler, once by the generic child visitor. Idempotent for acyclic mappings, but a cyclic mapping (a→b, b→a) swapped back and the leftover-validator threw:

AST transformation incomplete: found 1 untransformed schema name(s) "a" ...

Fix is a claim registry: each routable site (RangeVar / name-list array / (container, field)) is resolved through the router at most once per resultclaimSite(result, site, field) guarded inside transformRelation / transformNameList / transformSchemaNameField / rewriteSchemaField. Since the walker is pre-order, the most context-aware handler claims first (e.g. DropStmt routing ObjectWithArgs.objname with ns function beats the generic ObjectWithArgs visitor's ns unknown).

2. Deletes ~20 redundant statement handlers (pure duplicates of the generic RangeVar/TypeName/FuncCall visitors) and trims partial ones (CreateFunctionStmt return/param type loops, CreateTrigStmt.relation, AlterSeqStmt.sequence, CreateCastStmt type blocks, …). Handlers that carry real context stay — and several now pass their namespace instead of defaulting to unknown:

 AlterOwnerStmt / AlterObjectSchemaStmt / RenameStmt / CommentStmt:
-  transformNameList(node.object..., result)              // ns 'unknown'
+  transformNameList(node.object..., result, namespaceOfObjectType(...))

so object-level routes now apply to ALTER FUNCTION ... OWNER TO, ... SET SCHEMA, RENAME, and COMMENT ON forms (new test: a function-only route rewrites ALTER FUNCTION a.f(int) OWNER TO u).

3. Adds span to StatementFacts:

interface StatementFacts {
  ...
  span: StatementSpan; // { start, len } — parser byte offsets into the classified script
}

sql.slice(span.start, span.start + span.len) is the statement's verbatim source, so downstream consumers (pgpm/slice exclusion, the upcoming PgpmProgramAst statement layer) never need a second parse just to recover locations.

Net: transform.ts shrinks by ~200 lines; 234 tests green (228 existing + 6 new in routing-claims.test.ts); no API changes beyond the additive span field.

Link to Devin session: https://app.devin.ai/sessions/025fb88043964fdbb335ac5e39df2478
Requested by: @pyramation

@pyramation pyramation self-assigned this Jul 30, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@pyramation
pyramation merged commit 9dc2659 into main Jul 30, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant