fix: resolver fails to look up CTE columns#4430
Open
msaher wants to merge 1 commit intosqlc-dev:mainfrom
Open
Conversation
msaher
force-pushed
the
fix/4288
branch
5 times, most recently
from
a1eb004 to
cf988cd
Compare
Closes sqlc-dev#4228. The resolver fails to infer the type of a CTE column because it only searches normal tables. My solution is to have two passes, one where we search the CTE columns first (making them shadow regular tables) then if not found, we search through regular tables. We cannot simply add the CTE column in the existing `typeMap` because a CTE and a regular table will have the same column name which will trigger the `found > 1` check and the column will be considered ambiguous. So we use a different typeMap for CTEs so we can separate the search
msaher
commented
May 8, 2026
Comment on lines
-31
to
+30
| func (q *Queries) BadQuery(ctx context.Context, dollar_1 pgtype.Text) error { | ||
| _, err := q.db.Exec(ctx, badQuery, dollar_1) | ||
| func (q *Queries) BadQuery(ctx context.Context, name string) error { | ||
| _, err := q.db.Exec(ctx, badQuery, name) |
Author
There was a problem hiding this comment.
This change affects the existing cte_left_join test case. It now uses native Go types
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
1 participant
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.
Closes #4288
The resolver fails to infer the type of a CTE column because it only searches normal tables. My solution is to have two passes, one where we search the CTE columns first (making them shadow regular tables) then if not found, we search through regular tables.
We cannot simply add the CTE column in the existing
typeMapbecause a CTE and a regular table will have the same column name which will trigger thefound > 1check and the column will be considered ambiguous. So we use a different type map for CTEs so we can separate the searchI have noticed three other cases where only regular tables are searched. They are
ast.BetweenExpr,ast.ResTarget, andast.In. I'm not sure if they should be changed so I left them as they are and they're not failing any tests, but I added a comment about them in case we need to update them.There is some duplication when populating the CTEs and searching through them. I have chosen not to create a helper function for searching as that seems to be the style. However, I'm happy to create one if the maintainers think its needed.