---
phase: 27-alias-based-query-expansion
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
  - src/expand.rs
  - test/sql/phase27_qualified_refs.test
  - test/sql/TEST_LIST
autonomous: true
requirements: [EXP-01, EXP-05, CLN-02, CLN-03]

must_haves:
  truths:
    - "Qualified column references (alias.column) in dimension/metric expressions appear verbatim in generated SQL"
    - "The ON-clause substring matching join heuristic (resolve_joins) is deleted"
    - "resolve_joins_pkfk is the only join resolution path -- no has_pkfk conditional"
    - "Legacy definitions with empty fk_columns produce single-table queries (no JOINs) -- acceptable per no-backward-compat policy"
  artifacts:
    - path: "test/sql/phase27_qualified_refs.test"
      provides: "sqllogictest verifying dot-qualified expressions end-to-end"
      contains: "c.name"
    - path: "src/expand.rs"
      provides: "Cleaned expand module with only PK/FK join resolution"
  key_links:
    - from: "src/expand.rs"
      to: "src/graph.rs"
      via: "resolve_joins_pkfk calls RelationshipGraph::from_definition"
      pattern: "RelationshipGraph::from_definition"
---

<objective>
Verify qualified column references work end-to-end and remove the legacy join resolution code from expand.rs.

Purpose: EXP-05 needs an explicit test confirming `alias.column` expressions emit verbatim in SQL. CLN-03 removes the substring-matching `resolve_joins()` heuristic and the `has_pkfk` conditional, making `resolve_joins_pkfk()` the only join resolution path. EXP-01 and CLN-02 are pre-satisfied from Phase 26 but listed here for traceability.
Output: Clean expand.rs with no legacy join code, plus a new sqllogictest validating qualified column refs.
</objective>

<execution_context>
@/Users/paul/.claude/get-shit-done/workflows/execute-plan.md
@/Users/paul/.claude/get-shit-done/templates/summary.md
</execution_context>

<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/phases/27-alias-based-query-expansion/27-RESEARCH.md
@.planning/phases/26-pk-fk-join-resolution/26-02-SUMMARY.md

<interfaces>
<!-- Key types and contracts the executor needs. Extracted from codebase. -->

From src/expand.rs -- functions to DELETE (CLN-03):
```rust
// Lines ~169-235: DELETE THIS FUNCTION
fn resolve_joins<'a>(
    joins: &'a [Join],
    resolved_dims: &[&crate::model::Dimension],
    resolved_mets: &[&crate::model::Metric],
    def: &SemanticViewDefinition,
) -> Vec<&'a Join> { ... }

// Lines ~304-333: DELETE THIS FUNCTION
fn append_join_on_clause(sql: &mut String, join: &Join, def: &SemanticViewDefinition) { ... }
```

From src/expand.rs -- the has_pkfk conditional to simplify (lines ~500, 546-590):
```rust
// Line 500: DELETE this detection
let has_pkfk = def.joins.iter().any(|j| !j.fk_columns.is_empty());

// Lines 546-590: KEEP only the if-branch body (PK/FK path), DELETE else-branch
if has_pkfk {
    // Phase 26: Graph-based PK/FK join resolution -- KEEP THIS
    let ordered_aliases = resolve_joins_pkfk(def, &resolved_dims, &resolved_mets);
    // ...
} else {
    // Legacy path -- DELETE THIS ENTIRE BRANCH
    let needed_joins = resolve_joins(&def.joins, &resolved_dims, &resolved_mets, def);
    // ...
}
```

From src/expand.rs -- functions to KEEP:
```rust
fn resolve_joins_pkfk(def: &SemanticViewDefinition, ...) -> Vec<String>;
fn synthesize_on_clause(join: &Join, tables: &[TableRef]) -> String;
```

From test/sql/phase26_join_resolution.test -- existing qualified ref usage (passes):
```sql
DIMENSIONS (
    c.customer_name AS c.name     -- expr = "c.name" stored verbatim
)
METRICS (
    o.total_amount AS sum(o.amount) -- expr = "sum(o.amount)" stored verbatim
)
```
</interfaces>
</context>

<tasks>

<task type="auto" tdd="true">
  <name>Task 1: Add EXP-05 verification tests for qualified column references</name>
  <files>src/expand.rs, test/sql/phase27_qualified_refs.test, test/sql/TEST_LIST</files>
  <behavior>
    - Test 1 (Rust unit): expand() with a 2-table PK/FK definition where dim expr is "c.name" produces SQL containing "c.name AS" in the SELECT clause (not stripped or rewritten)
    - Test 2 (Rust unit): expand() with metric expr "sum(o.amount)" produces SQL containing "sum(o.amount) AS" verbatim
    - Test 3 (sqllogictest): CREATE SEMANTIC VIEW with qualified column refs in DIMENSIONS and METRICS, query returns correct data from the correct aliased tables
    - Test 4 (sqllogictest): Multiple qualified refs from different tables (e.g., c.name AND o.region) both resolve correctly in a single query
  </behavior>
  <action>
    1. In src/expand.rs test module, add a unit test `test_expand_qualified_column_refs_verbatim` that:
       - Builds a SemanticViewDefinition with 2 tables (o -> p27_orders, c -> p27_customers), PK/FK join (o.customer_id REFERENCES c), dimensions with expr "c.name", metrics with expr "sum(o.amount)"
       - Calls expand() and asserts the returned SQL string contains "c.name AS" and "sum(o.amount) AS" verbatim
       - This confirms EXP-05: expressions are emitted as-is, not modified

    2. Create test/sql/phase27_qualified_refs.test:
       - Create test tables p27_orders(id INT, customer_id INT, amount DECIMAL(10,2), region VARCHAR) and p27_customers(id INT, name VARCHAR)
       - Insert known data (3 orders across 2 customers + 1 order with NULL customer_id)
       - CREATE SEMANTIC VIEW p27_sales AS with qualified column refs in dims/metrics
       - Query with dimensions=['customer_name'] metrics=['total_amount'] and assert correct grouped results
       - Query with dimensions=['customer_name', 'order_region'] metrics=['total_amount'] to test multiple qualified refs from different tables
       - Use phase-prefixed table names (p27_) per project convention
       - Clean up with DROP SEMANTIC VIEW and DROP TABLE at end

    3. Add phase27_qualified_refs.test to test/sql/TEST_LIST
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test -- test_expand_qualified_column_refs_verbatim && just build && just test-sql</automated>
  </verify>
  <done>Rust unit test confirms expand() emits qualified expressions verbatim. sqllogictest confirms end-to-end correctness of qualified column references through the full extension pipeline.</done>
</task>

<task type="auto">
  <name>Task 2: Remove legacy join resolution code (CLN-03)</name>
  <files>src/expand.rs</files>
  <action>
    Delete the following from src/expand.rs:

    1. DELETE `resolve_joins()` function (lines ~169-235) -- the substring-matching transitive join resolver
    2. DELETE `append_join_on_clause()` function (lines ~304-333) -- the legacy ON clause builder
    3. DELETE `let has_pkfk = def.joins.iter().any(...)` line (~500)
    4. REMOVE the `if has_pkfk { ... } else { ... }` conditional (lines ~546-590) and KEEP only the PK/FK branch body (the `resolve_joins_pkfk` path). The PK/FK code should be unconditional -- just inline the body of the former if-branch.
    5. Add a brief comment where the conditional used to be: `// Join resolution via PK/FK graph (legacy resolve_joins removed in Phase 27).`
    6. Verify no remaining references to `resolve_joins` (non-pkfk version) or `append_join_on_clause` in the file.
    7. Remove the `use crate::model::Join` import if it is no longer used after deleting `resolve_joins` and `append_join_on_clause`. Check: `synthesize_on_clause` takes `&Join` so `Join` is still needed -- keep the import.
    8. Run `cargo test` to confirm all unit tests pass. Legacy expand unit tests that test the `has_pkfk=false` path will need updating or removal if they fail (they construct definitions with empty fk_columns). Check each failure: if it tests legacy join resolution behavior, delete it. If it tests general expand behavior, update it to use PK/FK-style definitions.

    Key safety check: `resolve_joins_pkfk()` returns `Vec::new()` for definitions with empty fk_columns (because `RelationshipGraph::from_definition` produces an empty graph). This means legacy single-table definitions still work (no JOINs needed). Legacy multi-table definitions with empty fk_columns will lose their JOINs -- this is acceptable per the no-backward-compat policy (STATE.md).
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test && just build && just test-sql</automated>
  </verify>
  <done>resolve_joins() and append_join_on_clause() are deleted. The has_pkfk conditional is removed. resolve_joins_pkfk() is the only join resolution path. All tests pass.</done>
</task>

</tasks>

<verification>
- `cargo test` passes (all Rust unit + prop tests)
- `just build && just test-sql` passes (all sqllogictest files)
- `grep -n "fn resolve_joins\b" src/expand.rs` returns no matches (function deleted)
- `grep -n "fn append_join_on_clause" src/expand.rs` returns no matches (function deleted)
- `grep -n "has_pkfk" src/expand.rs` returns no matches (conditional removed)
- `test/sql/phase27_qualified_refs.test` exists and is listed in TEST_LIST
</verification>

<success_criteria>
- EXP-05: A Rust unit test asserts expand() emits qualified column refs verbatim, and a sqllogictest confirms end-to-end correctness
- CLN-03: resolve_joins() and append_join_on_clause() are deleted; resolve_joins_pkfk() is the only join resolution path
- EXP-01 and CLN-02: Pre-satisfied from Phase 26 (no new work needed)
- All tests pass via `cargo test` and `just test-sql`
</success_criteria>

<output>
After completion, create `.planning/phases/27-alias-based-query-expansion/27-01-SUMMARY.md`
</output>
