feat(transform): route object identities by cast type - #343
Merged
Conversation
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
'sch.tbl'::regclassis not opaque text — it is a reference the parser resolves, and the cast says which namespace it resolves in. The transform previously treated it as a plain string constant and rewrote it withtransformSchemaRefsInString, i.e. "does this text contain<mappedSchema>.". Two consequences: object-level routes and rebinds never applied to it (only the schema-level map did), and a reference with no dot was unreachable —'apps_private'::regnamespacecannot be matched at all, which is why downstream tooling still keeps helper-name lists (verify_(function|table|...)regexes) purely to rename bare schema names.This adds a
TypeCastvisitor keyed on the cast's target type, so those references route through the sameSchemaRouterpath as aRangeVar:regnamespacemeans the whole operand is a schema name;regclass/regproc/regprocedure/regtypemean the qualifier before the first dot is, with the object name routed in its own namespace (regprocedure's argument list is preserved verbatim). Both scoped and unqualified operands are handled — an unqualified'users'::regclassnames no schema (search_pathresolves it), so it is returned unchanged rather than guessed at. Target names are re-emitted throughQuoteUtils.quoteIdentifier, so a hyphenated route yields'"my-schema".users'::regclass.Because the key is the cast and not the calling function's name, nothing needs registering when a new helper appears — the failure mode where
verify_securitywas omitted from the regex list cannot recur.The generic
A_Constvisitor now claims its site (claimSite(result, node.sval, 'sval')) so a literal reached by both visitors routes exactly once; without that a cyclic mapping (a→b, b→a) swaps back.New exports:
identityCastNamespace,transformIdentityCastLiteral.transformVerifyCallsis left in place — it can be deleted once its consumers emit identity casts.Link to Devin session: https://app.devin.ai/sessions/37a60838adfb4fd6ac8c29f09c2e2827
Requested by: @pyramation