---
phase: 24-pk-fk-model
plan: 02
type: execute
wave: 2
depends_on:
  - 24-01
files_modified:
  - src/ddl/define.rs
  - src/ddl/parse_args.rs
  - test/sql/phase2_ddl.test
  - test/sql/phase4_query.test
autonomous: true
requirements:
  - DDL-06

must_haves:
  truths:
    - "Function-based create_semantic_view() accepts tables with pk_columns parameter"
    - "Function-based create_semantic_view() accepts relationships with name, from_alias, to_alias, fk_columns parameters"
    - "Qualified dimension/metric names (alias.name) auto-populate source_table when source_table is empty"
    - "All existing sqllogictest tests pass with updated relationship syntax"
    - "just test-all passes (full suite including sqllogictest and DuckLake CI)"
  artifacts:
    - path: "src/ddl/define.rs"
      provides: "Updated named_parameters() with pk_columns in tables and PK/FK relationship struct"
      contains: "pk_columns"
    - path: "src/ddl/parse_args.rs"
      provides: "Updated extraction logic for new struct fields and qualified name parsing"
      contains: "fk_columns"
    - path: "test/sql/phase2_ddl.test"
      provides: "Updated relationship syntax in DDL tests"
      contains: "from_alias"
    - path: "test/sql/phase4_query.test"
      provides: "Updated relationship syntax in query tests"
      contains: "from_alias"
  key_links:
    - from: "src/ddl/define.rs::named_parameters"
      to: "src/ddl/parse_args.rs::parse_define_args_from_bind"
      via: "Struct field order must match (index 0=name, 1=from_alias, 2=to_alias, 3=fk_columns)"
      pattern: "named_parameters"
    - from: "src/ddl/parse_args.rs"
      to: "src/model.rs::Join"
      via: "Constructs Join with new from_alias, fk_columns, name fields"
      pattern: "from_alias"
    - from: "src/ddl/parse_args.rs"
      to: "src/model.rs::TableRef"
      via: "Constructs TableRef with new pk_columns field"
      pattern: "pk_columns"
---

<objective>
Update the function-based DDL interface to accept PK/FK model parameters, including pk_columns on tables, from_alias/fk_columns/name on relationships, and qualified name parsing for dimensions/metrics.

Purpose: The function-based `create_semantic_view()` path must accept the same PK/FK model as the SQL DDL path (Phase 25). This is a breaking change to the function interface, which is acceptable per STATE.md ("NO backward compatibility needed").

Output: Updated type signatures, extraction logic, and migrated sqllogictest files.
</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/24-pk-fk-model/24-RESEARCH.md
@.planning/phases/24-pk-fk-model/24-01-SUMMARY.md
@src/ddl/define.rs
@src/ddl/parse_args.rs
@src/model.rs

<interfaces>
<!-- From Plan 01: the new model struct fields the executor needs to construct -->

From src/model.rs (after Plan 01):
```rust
pub struct TableRef {
    pub alias: String,
    pub table: String,
    #[serde(default)]
    pub pk_columns: Vec<String>,
}

pub struct Join {
    pub table: String,
    #[serde(default, skip_serializing_if = "String::is_empty")]
    pub on: String,
    #[serde(default)]
    pub from_cols: Vec<String>,
    #[serde(default)]
    pub join_columns: Vec<JoinColumn>,
    #[serde(default)]
    pub from_alias: String,
    #[serde(default)]
    pub fk_columns: Vec<String>,
    #[serde(default)]
    pub name: Option<String>,
}
```

<!-- Current define.rs named_parameters (to be replaced) -->

From src/ddl/define.rs (current named_parameters):
```rust
fn named_parameters() -> Option<Vec<(String, LogicalTypeHandle)>> {
    // tables: LIST(STRUCT(alias VARCHAR, table VARCHAR))
    // relationships: LIST(STRUCT(from_table VARCHAR, to_table VARCHAR, join_columns LIST(STRUCT(from VARCHAR, to VARCHAR))))
    // dimensions: LIST(STRUCT(name VARCHAR, expr VARCHAR, source_table VARCHAR))
    // metrics: LIST(STRUCT(name VARCHAR, expr VARCHAR, source_table VARCHAR))
}
```

<!-- Current parse_args.rs struct field extraction order (to be updated) -->

From src/ddl/parse_args.rs (current extraction):
```rust
// Tables: index 0=alias, 1=table
// Relationships: index 0=from_table, 1=to_table, 2=join_columns(LIST(STRUCT(from, to)))
// Dimensions: index 0=name, 1=expr, 2=source_table
// Metrics: index 0=name, 1=expr, 2=source_table
```
</interfaces>
</context>

<tasks>

<task type="auto">
  <name>Task 1: Update named_parameters() and parse_args extraction for PK/FK model</name>
  <files>src/ddl/define.rs, src/ddl/parse_args.rs</files>
  <action>
**In `src/ddl/define.rs` -- update `named_parameters()`:**

1. Change the `tables_type` to include `pk_columns`:
```rust
let pk_columns_type = LogicalTypeHandle::list(&varchar());
let tables_type = LogicalTypeHandle::list(&LogicalTypeHandle::struct_type(&[
    ("alias", varchar()),
    ("table", varchar()),
    ("pk_columns", pk_columns_type),
]));
```

2. Replace the `relationships_type` entirely. Remove the old `col_pair_struct` and `join_columns_type` variables. The new relationships struct uses PK/FK model:
```rust
let fk_columns_type = LogicalTypeHandle::list(&varchar());
let relationships_type = LogicalTypeHandle::list(&LogicalTypeHandle::struct_type(&[
    ("name", varchar()),
    ("from_alias", varchar()),
    ("to_alias", varchar()),
    ("fk_columns", fk_columns_type),
]));
```

3. Leave `dimensions_type` and `metrics_type` UNCHANGED (they already have source_table).

4. Update the module-level doc comment on `parse_args.rs` to reflect the new type signatures:
   - tables: `LIST(STRUCT(alias VARCHAR, table VARCHAR, pk_columns LIST(VARCHAR)))`
   - relationships: `LIST(STRUCT(name VARCHAR, from_alias VARCHAR, to_alias VARCHAR, fk_columns LIST(VARCHAR)))`

**In `src/ddl/parse_args.rs` -- update extraction logic:**

5. **Tables extraction (Param 1):** Add pk_columns extraction after alias and table. The pk_columns are at struct child index 2 and are a `LIST(VARCHAR)`:
```rust
// After extracting alias (index 0) and table (index 1):
let mut pk_val = ffi::duckdb_get_struct_child(child, 2); // pk_columns
let pk_size = ffi::duckdb_get_list_size(pk_val);
let mut pk_columns: Vec<String> = Vec::new();
for j in 0..pk_size {
    let mut pk_child = ffi::duckdb_get_list_child(pk_val, j);
    pk_columns.push(extract_varchar(pk_child));
    ffi::duckdb_destroy_value(&mut pk_child);
}
ffi::duckdb_destroy_value(&mut pk_val);
// Then: tables.push(TableRef { alias, table, pk_columns });
```

6. **Relationships extraction (Param 2):** Replace the entire relationships extraction block. The new struct has 4 fields at indices 0-3:
   - Index 0: `name` (VARCHAR) -- relationship name, empty string = unnamed
   - Index 1: `from_alias` (VARCHAR) -- source table alias
   - Index 2: `to_alias` (VARCHAR) -- target table alias (resolved to physical table via tables vec)
   - Index 3: `fk_columns` (LIST(VARCHAR)) -- FK column names on the source side

```rust
let mut rel_child = ffi::duckdb_get_list_child(val_ptr, i);
let rel_name_str = extract_struct_child_varchar(rel_child, 0); // name
let from_alias = extract_struct_child_varchar(rel_child, 1);    // from_alias
let to_alias_str = extract_struct_child_varchar(rel_child, 2);  // to_alias

// Resolve to_alias to physical table name via tables vec
let to_table = tables
    .iter()
    .find(|t| t.alias.eq_ignore_ascii_case(&to_alias_str))
    .map(|t| t.table.clone())
    .unwrap_or(to_alias_str);

// Extract fk_columns: LIST(VARCHAR) at index 3
let mut fk_val = ffi::duckdb_get_struct_child(rel_child, 3);
let fk_size = ffi::duckdb_get_list_size(fk_val);
let mut fk_columns: Vec<String> = Vec::new();
for j in 0..fk_size {
    let mut fk_child = ffi::duckdb_get_list_child(fk_val, j);
    fk_columns.push(extract_varchar(fk_child));
    ffi::duckdb_destroy_value(&mut fk_child);
}
ffi::duckdb_destroy_value(&mut fk_val);
ffi::duckdb_destroy_value(&mut rel_child);

let rel_name = if rel_name_str.is_empty() { None } else { Some(rel_name_str) };

joins.push(Join {
    table: to_table,
    on: String::new(),
    from_cols: vec![],
    join_columns: vec![],
    from_alias,
    fk_columns,
    name: rel_name,
});
```

7. **Dimensions extraction (Param 3):** Add qualified name parsing. After extracting `dim_name` and `source_table_str`, if `source_table_str` is empty and `dim_name` contains a dot, split on first dot to derive source_table:
```rust
let (final_name, source_table) = if source_table_str.is_empty() {
    if let Some(dot_pos) = dim_name.find('.') {
        let alias = dim_name[..dot_pos].to_string();
        let bare = dim_name[dot_pos + 1..].to_string();
        (bare, Some(alias))
    } else {
        (dim_name, None)
    }
} else {
    (dim_name, Some(source_table_str))
};
// Use final_name and source_table in the Dimension constructor
```

8. **Metrics extraction (Param 4):** Apply the same qualified name parsing logic as dimensions (step 7).

9. **CRITICAL field order warning:** The struct child index in parse_args.rs MUST match the field declaration order in named_parameters(). Document the index-to-field mapping with comments at each extraction point. For example:
   - Tables: `0=alias, 1=table, 2=pk_columns`
   - Relationships: `0=name, 1=from_alias, 2=to_alias, 3=fk_columns`

10. Remove the `JoinColumn` import from `parse_args.rs` use statement if it is no longer used in the extraction code. Keep `JoinColumn` in `model.rs` (it is still used by existing stored definitions and expand.rs).
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test -x</automated>
  </verify>
  <done>
    - `named_parameters()` declares tables with pk_columns and relationships with name/from_alias/to_alias/fk_columns
    - `parse_args.rs` extracts new struct fields with correct index-to-field mapping
    - Qualified name parsing splits `alias.name` to set source_table on dims/metrics
    - `cargo test` passes (existing unit tests; sqllogictest may fail until Task 2)
  </done>
</task>

<task type="auto">
  <name>Task 2: Migrate sqllogictest files to new relationship syntax and run full suite</name>
  <files>test/sql/phase2_ddl.test, test/sql/phase4_query.test</files>
  <action>
Update all `create_semantic_view()` calls in sqllogictest files to use the new relationship struct format.

**Old format (to find and replace):**
```sql
relationships := [{'from_table': 'o', 'to_table': 'c',
  'join_columns': [{'from': 'customer_id', 'to': 'id'}]}]
```

**New format (replacement):**
```sql
relationships := [{'name': '', 'from_alias': 'o', 'to_alias': 'c',
  'fk_columns': ['customer_id']}]
```

Key changes:
- `from_table` -> `from_alias` (same value -- it was already an alias)
- `to_table` -> `to_alias` (same value -- it was already an alias)
- `join_columns: [{'from': 'X', 'to': 'Y'}]` -> `fk_columns: ['X']` (only FK column, PK inferred from target table's pk_columns)
- Add `'name': ''` (empty string = unnamed relationship)

**Also update tables to include pk_columns:**
```sql
-- Old:
tables := [{'alias': 'o', 'table': 'orders'}, {'alias': 'c', 'table': 'customers'}]
-- New:
tables := [{'alias': 'o', 'table': 'orders', 'pk_columns': ['id']},
           {'alias': 'c', 'table': 'customers', 'pk_columns': ['id']}]
```

Note: The PK values should match what the FK references. For the existing test tables:
- `test/sql/phase2_ddl.test` line ~182: orders(o) -> customers(c) via customer_id. Customers PK is `id`, orders PK is `id`.
- `test/sql/phase4_query.test` line ~139: test_orders(o) -> test_customers(c) via customer_id (PK: id), test_orders(o) -> test_products(p) via category (PK: category).

**Files to update:**

1. `test/sql/phase2_ddl.test`:
   - Line ~182-188: Update the multi-table view test (test 12). Change tables to include pk_columns, relationships to new format.
   - Line ~271: `relationships := []` -- this is fine as-is (empty list).

2. `test/sql/phase4_query.test`:
   - Line ~139-153: Update the joined_orders definition. Change tables to include pk_columns, relationships to new format.

After updating both files, run the full test suite to confirm everything passes:
```bash
just build && just test-all
```

If any tests fail, fix the test data to match the actual table schemas used in the test fixtures.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && just build && just test-all</automated>
  </verify>
  <done>
    - All sqllogictest files use new relationship syntax (name, from_alias, to_alias, fk_columns)
    - All sqllogictest files use tables with pk_columns
    - `just test-all` passes: cargo test (unit + proptest), sqllogictest, DuckLake CI
    - No references to old `from_table`/`to_table`/`join_columns` in sqllogictest files (except in test descriptions/comments)
  </done>
</task>

</tasks>

<verification>
```bash
cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && just test-all
```
Full test suite passes: Rust unit tests, property-based tests, SQL logic tests, DuckLake CI.
</verification>

<success_criteria>
- `create_semantic_view()` function accepts tables with `pk_columns` parameter
- `create_semantic_view()` function accepts relationships with `name`, `from_alias`, `to_alias`, `fk_columns` parameters
- Qualified dimension/metric names (`alias.name`) auto-populate `source_table`
- `just test-all` passes with 0 failures
</success_criteria>

<output>
After completion, create `.planning/phases/24-pk-fk-model/24-02-SUMMARY.md`
</output>
