---
phase: 11.1-review-possible-ddl-and-query-syntax-options-and-bring-it-as-close-as-we-can-to-snowflake-semantic-views
plan: "03"
type: execute
wave: 2
depends_on: ["01"]
files_modified:
  - src/expand.rs
autonomous: true
requirements:
  - DDL-04

must_haves:
  truths:
    - "expand() generates JOIN ON clause from Join.join_columns when join_columns is non-empty"
    - "expand() falls back to Join.on when join_columns is empty (backward compat)"
    - "Table-qualified dimension names ('o.region') resolve correctly via alias prefix lookup"
    - "Table-qualified metric names ('o.revenue') resolve correctly"
    - "Unqualified names still resolve as before (backward compat)"
    - "cargo test passes all expand.rs tests including new ones"
  artifacts:
    - path: "src/expand.rs"
      provides: "Updated join codegen + table-qualified name resolution"
  key_links:
    - from: "src/expand.rs"
      to: "src/query/table_function.rs"
      via: "expand() called from SemanticViewVTab bind()"
---

<objective>
Update expand.rs to use the new Join.join_columns model for SQL codegen and to support table-qualified dimension/metric names in query requests.

Purpose: The new DDL writes join_columns (column-pair structs) instead of a raw ON clause string. expand() must generate the JOIN ON clause from these pairs. Also, the renamed semantic_view function adds support for 'o.region' style names in dimensions/metrics arrays — these are lookup keys that include the source_table alias prefix.

Output: Updated src/expand.rs with backward-compatible JOIN codegen and table-qualified name resolution. All existing tests pass plus new tests.
</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/phases/11.1-review-possible-ddl-and-query-syntax-options-and-bring-it-as-close-as-we-can-to-snowflake-semantic-views/11.1-CONTEXT.md
@.planning/phases/11.1-review-possible-ddl-and-query-syntax-options-and-bring-it-as-close-as-we-can-to-snowflake-semantic-views/11.1-RESEARCH.md
@.planning/phases/11.1-review-possible-ddl-and-query-syntax-options-and-bring-it-as-close-as-we-can-to-snowflake-semantic-views/11.1-01-SUMMARY.md

<interfaces>
<!-- From Plan 01 model.rs additions -->
```rust
pub struct JoinColumn { pub from: String, pub to: String }

pub struct Join {
    pub table: String,
    pub on: String,                      // legacy: raw ON clause (kept for serde compat)
    pub from_cols: Vec<String>,          // deprecated Phase 11 format
    pub join_columns: Vec<JoinColumn>,   // Phase 11.1: column-pair structs
}
```

<!-- Current expand.rs join SQL codegen (lines ~323-328) -->
```rust
for join in &needed_joins {
    sql.push_str("\n    JOIN ");
    sql.push_str(&quote_table_ref(&join.table));
    sql.push_str(" ON ");
    sql.push_str(&join.on);    // <-- THIS must be updated
}
```

<!-- Current find_dimension function -->
```rust
fn find_dimension<'a>(def: &'a SemanticViewDefinition, name: &str) -> Option<&'a Dimension> {
    def.dimensions.iter().find(|d| d.name.eq_ignore_ascii_case(name))
}
```

<!-- Current resolve_joins transitive resolution uses join.on -->
// The transitive join resolver in resolve_joins() checks join.on for references to other tables.
// When join_columns is used (no raw ON clause), the transitive resolution via on-clause text
// scanning is irrelevant — just add all declared joins in that case, or leave as-is since
// new-style joins won't have ON-clause-based transitive deps. Keep the existing logic; it
// simply won't match anything for new-style joins (join.on will be empty string).

<!-- CONTEXT.md: Table-qualified dim names -->
// 'o.region' → split on first '.': alias='o', bare_name='region'
// Look up dimension by bare_name AND source_table == alias (case-insensitive)
// Fallback: if no match with alias, try bare_name lookup only (backward compat for existing code)
// Error message should include full 'o.region' as the "unknown" name
</interfaces>
</context>

<tasks>

<task type="auto" tdd="true">
  <name>Task 1: Update JOIN codegen to use join_columns; add table-qualified name resolution</name>
  <files>src/expand.rs</files>
  <behavior>
    JOIN codegen tests:
    - Test A: Join with join_columns=[JoinColumn{from:"customer_id",to:"id"}] + tables=[TableRef{alias:"o",table:"orders"},TableRef{alias:"c",table:"customers"}] generates `JOIN "customers" ON "o"."customer_id" = "c"."id"` (or equivalent alias-qualified form)
    - Test B: Join with join_columns containing 2 pairs generates `JOIN "tbl" ON "a"."col1" = "b"."col1" AND "a"."col2" = "b"."col2"`
    - Test C: Join with empty join_columns but non-empty on falls back to `JOIN "tbl" ON {on}` (legacy behavior unchanged)
    - Test D: All existing join tests still pass (backward compat)

    Table-qualified name resolution tests:
    - Test E: dimensions := ['o.region'] where a Dimension named 'region' has source_table=Some('o') resolves correctly
    - Test F: dimensions := ['region'] (no prefix) still resolves by bare name (backward compat)
    - Test G: dimensions := ['o.region'] where no dimension named 'region' has source_table='o' returns UnknownDimension error with 'o.region' as the name
    - Test H: dimensions := ['c.tier'] resolves against source_table='c' correctly
    - Test I: metrics := ['o.revenue'] where metric 'revenue' has source_table=Some('o') resolves correctly
  </behavior>
  <action>
Make the following targeted changes to src/expand.rs:

**Change 1: Update join SQL generation in expand()**

Find the loop that emits JOIN clauses (around line 323):
```rust
for join in &needed_joins {
    sql.push_str("\n    JOIN ");
    sql.push_str(&quote_table_ref(&join.table));
    sql.push_str(" ON ");
    sql.push_str(&join.on);
}
```

Replace with logic that picks join_columns if non-empty, else falls back to on:
```rust
for join in &needed_joins {
    sql.push_str("\n    JOIN ");
    sql.push_str(&quote_table_ref(&join.table));
    sql.push_str(" ON ");
    if !join.join_columns.is_empty() {
        // New-style: generate ON clause from column-pair structs.
        // Determine the from_table alias and to_table alias from SemanticViewDefinition.tables.
        // The join.table is the physical name of the joined table; look up its alias in def.tables.
        // The base table alias is def.tables[0].alias if tables is non-empty.
        //
        // Pattern: "from_alias"."from_col" = "to_alias"."to_col" AND ...
        // We need the from_alias (base/from table) and to_alias (joined table).
        // Look up alias for join.table in def.tables; if not found, use join.table as alias.
        let to_alias = def.tables.iter()
            .find(|t| t.table.eq_ignore_ascii_case(&join.table))
            .map(|t| t.alias.as_str())
            .unwrap_or(&join.table);
        // The from table is the base table — its alias is def.tables first entry's alias, or base_table name.
        let from_alias = def.tables.first()
            .map(|t| t.alias.as_str())
            .unwrap_or(&def.base_table);
        let on_parts: Vec<String> = join.join_columns.iter().map(|jc| {
            format!("{}.{} = {}.{}",
                quote_ident(from_alias), quote_ident(&jc.from),
                quote_ident(to_alias), quote_ident(&jc.to))
        }).collect();
        sql.push_str(&on_parts.join(" AND "));
    } else {
        // Legacy: raw ON clause string (Phase 10 and earlier definitions).
        sql.push_str(&join.on);
    }
}
```

Note: The expand() function signature must also accept `def` in scope (it already does — def is the SemanticViewDefinition parameter). Pass def through to the join loop. This requires no signature change since expand() already takes `def: &SemanticViewDefinition`.

**Change 2: Update find_dimension to support table-qualified names**

Replace the current `find_dimension` function with one that handles 'alias.name' lookup:
```rust
/// Look up a dimension by name using case-insensitive matching.
///
/// Supports table-qualified names: if `name` contains a '.' (e.g., "o.region"),
/// splits into (alias, bare_name) and also matches source_table == alias.
/// Falls back to bare_name lookup if no qualified match is found.
fn find_dimension<'a>(
    def: &'a SemanticViewDefinition,
    name: &str,
) -> Option<&'a crate::model::Dimension> {
    if let Some(dot_pos) = name.find('.') {
        let alias = &name[..dot_pos];
        let bare = &name[dot_pos + 1..];
        // Try qualified lookup: bare_name match AND source_table == alias
        if let Some(d) = def.dimensions.iter().find(|d| {
            d.name.eq_ignore_ascii_case(bare)
                && d.source_table.as_deref().map(|st| st.eq_ignore_ascii_case(alias)).unwrap_or(false)
        }) {
            return Some(d);
        }
        // Fall back to bare_name only (for forward-compat with future changes)
        def.dimensions.iter().find(|d| d.name.eq_ignore_ascii_case(bare))
    } else {
        def.dimensions.iter().find(|d| d.name.eq_ignore_ascii_case(name))
    }
}
```

**Change 3: Similarly update find_metric for table-qualified names**

```rust
fn find_metric<'a>(
    def: &'a SemanticViewDefinition,
    name: &str,
) -> Option<&'a crate::model::Metric> {
    if let Some(dot_pos) = name.find('.') {
        let alias = &name[..dot_pos];
        let bare = &name[dot_pos + 1..];
        if let Some(m) = def.metrics.iter().find(|m| {
            m.name.eq_ignore_ascii_case(bare)
                && m.source_table.as_deref().map(|st| st.eq_ignore_ascii_case(alias)).unwrap_or(false)
        }) {
            return Some(m);
        }
        def.metrics.iter().find(|m| m.name.eq_ignore_ascii_case(bare))
    } else {
        def.metrics.iter().find(|m| m.name.eq_ignore_ascii_case(name))
    }
}
```

**Change 4: Duplicate detection for qualified names**

The `seen_dims` HashSet currently uses `name.to_ascii_lowercase()` for the full string (including any 'o.' prefix). This is correct — 'o.region' and 'region' would be treated as distinct, which is acceptable behavior. No change needed here.

**Change 5: Error message for qualified names**

In the UnknownDimension error, the `name` field is already the full requested string (e.g., 'o.region') — error messages will naturally show the qualified name. No extra change needed.

**Add new unit tests** in expand_tests (Phase 11.1 sub-module):
- Test join_columns generates correct ON clause
- Test multi-column join_columns generates AND-joined ON clause
- Test join with empty join_columns falls back to on string
- Test table-qualified dimension lookup succeeds with matching source_table
- Test table-qualified metric lookup succeeds
- Test bare name still resolves (backward compat)
- Test unknown qualified name returns UnknownDimension with full 'alias.name' as the name

Run `cargo test` after changes.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test 2>&1 | tail -20</automated>
  </verify>
  <done>cargo test passes; expand() generates alias-qualified ON clauses from join_columns; 'o.region' style dimension names resolve correctly; all pre-existing expand tests still pass</done>
</task>

</tasks>

<verification>
`cargo test` passes all existing expand tests plus new Phase 11.1 tests. No regression in join resolution or dimension lookup.
</verification>

<success_criteria>
- JOIN SQL codegen uses join_columns when non-empty; falls back to join.on when empty
- find_dimension and find_metric support 'alias.name' qualified lookup
- All pre-existing expand.rs tests pass
- New tests cover join_columns codegen and table-qualified name resolution
</success_criteria>

<output>
After completion, create `.planning/phases/11.1-review-possible-ddl-and-query-syntax-options-and-bring-it-as-close-as-we-can-to-snowflake-semantic-views/11.1-03-SUMMARY.md`
</output>
