Skip to content

feat(parser/postgres): wire INNER JOIN queries through multi-table resolver - #47

Closed
JagritGumber wants to merge 2 commits into
feat/parser-join-resolver-scaffoldingfrom
feat/postgres-parser-inner-join
Closed

feat(parser/postgres): wire INNER JOIN queries through multi-table resolver#47
JagritGumber wants to merge 2 commits into
feat/parser-join-resolver-scaffoldingfrom
feat/postgres-parser-inner-join

Conversation

@JagritGumber

@JagritGumber JagritGumber commented Apr 19, 2026

Copy link
Copy Markdown
Owner

Summary

Second PR in the JOIN series. Wires the postgres parser to use the multi-table resolver from #46 when a query contains a JOIN keyword.

Base branch is set to #46 (`feat/parser-join-resolver-scaffolding`). When #46 merges to main, this PR's base will auto-retarget to main and only this PR's commits will show in the diff.

Behavior changes

Queries with JOIN now work:
```sql
-- name: GetUserWithOrg :one
SELECT users.name, orgs.slug
FROM users INNER JOIN orgs ON users.org_id = orgs.id
WHERE users.id = $1;
```
Returns a `QueryDef` where each column has `source_table` populated. Codegen will consume this in the final PR of the series (C16).

Still rejected, with clear v1.2-roadmap messages:

  • `LEFT` / `RIGHT` / `FULL OUTER JOIN` (needs nullability propagation)
  • `USING (col)`, `NATURAL JOIN`, `CROSS JOIN`
  • `SELECT *` across joined tables
  • Qualified selects on single-table queries (that's PR feat: support qualified single-table select columns #32's scope, untouched here)

Changes

  • `resolve_return_columns` gains a `&[TableDef]` param (schema tables)
  • New `JOIN_DETECT_RE` gate
  • New `resolve_return_columns_multi_table` helper that uses `parse_join_clauses` + `resolve_multi_table_select_column`

Test plan

  • 5 new postgres parser unit tests (qualified cols, AS aliases, SELECT * rejection, LEFT JOIN rejection, single-table rejection still active)
  • Updated the CLI integration test that was asserting the old rejection — it now asserts JOIN queries succeed
  • `cargo test --workspace` — 158 lib + 11 CLI + 2 e2e + 4 typecheck all pass
  • `cargo clippy --workspace --all-targets -- -D warnings` — clean

Open in Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

JagritGumber added a commit that referenced this pull request Apr 19, 2026
Mirror PR #47 (postgres) for the remaining two dialects. Both
mysql.rs and sqlite.rs gain:

- A `&[TableDef]` schema_tables param on resolve_return_columns
- JOIN_DETECT_RE check that routes to the multi-table resolver path
- 3 new unit tests per dialect (qualified columns, SELECT *
  rejection, LEFT JOIN rejection)

Also lift the resolve_return_columns_multi_table helper that lived
in postgres.rs into parser/joins.rs as resolve_multi_table_columns,
shared across all three dialects to avoid copy-paste. Same goes
for the JOIN_DETECT_RE constant — now `pub` from joins.rs.

164 lib + 11 CLI + 2 e2e + 4 typecheck tests all pass. Clippy clean.

After this PR all three dialects accept INNER JOIN queries with
qualified columns. Codegen consuming source_table is the final
step (C16).
…solver

When a query contains the JOIN keyword, route each SELECT column through
the multi-table resolver from parser::joins (landed in the prior PR).
Qualified columns now resolve across the alias map and land in QueryDef.returns
with source_table populated.

Queries without JOIN keep the existing single-table path unchanged —
ensure_supported_select_expr still rejects qualified selects there.
Lifting that restriction for single-table queries is a separate effort
(PR #32 was in flight before this series).

Changes:
- resolve_return_columns gains a &[TableDef] schema_tables param
- JOIN_DETECT_RE gates multi-table resolution
- resolve_return_columns_multi_table rejects SELECT * across joins with
  a clear v1.2 pointer
- Parser propagates schema_tables from parse_queries to resolve_return_columns

Tests:
- 5 new postgres parser unit tests (qualified columns, AS aliases, SELECT *
  rejection, LEFT JOIN rejection, single-table rejection still active)
- Updated the cli test that was asserting the old rejection — it now
  asserts the JOIN query succeeds (renamed from cli_generate_rejects_
  qualified_selects to cli_generate_accepts_multi_table_inner_join)

158 lib + 11 CLI + 2 e2e + 4 typecheck tests all pass. Clippy clean.
…ives

Devin flagged that `JOIN_DETECT_RE.is_match(sql)` matches `\bJOIN\b`
anywhere in the SQL, which false-triggers on queries where the JOIN
lives inside a subquery (e.g. `WHERE id IN (SELECT ... JOIN ...)`).
The outer query's single-table SELECT was then misrouted to the
multi-table resolver and failed with a confusing error.

Fix: route through `parser::joins::has_outer_join`, which scopes the
check to the outer FROM body via FROM_CLAUSE_RE. JOINs inside
subqueries stay behind the WHERE boundary and no longer trip the
outer detection.

Also drop the local JOIN_DETECT_RE static (was unused after the
switch) and the stale imports. New regression test covers the
subquery case end-to-end through parse_queries.
@JagritGumber
JagritGumber force-pushed the feat/postgres-parser-inner-join branch from 06917c9 to 4ab2d7d Compare April 20, 2026 00:17
JagritGumber added a commit that referenced this pull request Apr 20, 2026
Mirror PR #47 (postgres) for the remaining two dialects. Both
mysql.rs and sqlite.rs gain:

- A `&[TableDef]` schema_tables param on resolve_return_columns
- JOIN_DETECT_RE check that routes to the multi-table resolver path
- 3 new unit tests per dialect (qualified columns, SELECT *
  rejection, LEFT JOIN rejection)

Also lift the resolve_return_columns_multi_table helper that lived
in postgres.rs into parser/joins.rs as resolve_multi_table_columns,
shared across all three dialects to avoid copy-paste. Same goes
for the JOIN_DETECT_RE constant — now `pub` from joins.rs.

164 lib + 11 CLI + 2 e2e + 4 typecheck tests all pass. Clippy clean.

After this PR all three dialects accept INNER JOIN queries with
qualified columns. Codegen consuming source_table is the final
step (C16).
@JagritGumber
JagritGumber deleted the branch feat/parser-join-resolver-scaffolding April 20, 2026 00:26
@JagritGumber
JagritGumber deleted the feat/postgres-parser-inner-join branch April 20, 2026 00:48
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