---
phase: 17.1-python-vtab-crash-investigation
plan: 02
type: execute
wave: 2
depends_on:
  - "17.1-01"
files_modified:
  - src/query/table_function.rs
  - test/integration/test_vtab_crash.py
autonomous: true
requirements:
  - CRASH-03
  - CRASH-04
  - CRASH-05

must_haves:
  truths:
    - "Python crash reproduction script passes all tests (no CRASH or FAIL results)"
    - "Existing CLI sqllogictest suite passes (just test-sql)"
    - "Existing Rust unit/property tests pass (cargo test)"
    - "DuckLake CI integration tests pass (just test-ducklake-ci)"
  artifacts:
    - path: "src/query/table_function.rs"
      provides: "Fixed zero-copy vector transfer with runtime type validation"
      contains: "duckdb_vector_get_column_type"
    - path: "test/integration/test_vtab_crash.py"
      provides: "Updated crash reproduction script (all tests pass post-fix)"
      min_lines: 100
  key_links:
    - from: "src/query/table_function.rs"
      to: "duckdb_vector_reference_vector"
      via: "runtime type check before zero-copy reference"
      pattern: "duckdb_vector_get_column_type|duckdb_get_type_id"
---

<objective>
Fix the identified crash root cause and verify no regressions across the full test suite.

Purpose: Apply a targeted fix for the crash vector identified in Plan 01, then verify the fix resolves the crash while preserving all existing functionality.
Output: Fixed source code, green crash reproduction tests, and full test suite passing.
</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/17.1-python-vtab-crash-investigation/17.1-RESEARCH.md
@.planning/phases/17.1-python-vtab-crash-investigation/17.1-01-SUMMARY.md

@src/query/table_function.rs
@src/lib.rs
</context>

<interfaces>
<!-- Key types and contracts the executor needs from table_function.rs -->

From src/query/table_function.rs (zero-copy transfer loop, lines 587-598):
```rust
// Zero-copy transfer: reference each source vector into the output chunk.
let out_ptr = output.get_ptr();
let n_cols = streaming.col_count.min(bind_data.column_names.len());
for col_idx in 0..n_cols {
    let src_vec = unsafe { ffi::duckdb_data_chunk_get_vector(src_chunk, col_idx as ffi::idx_t) };
    let dst_vec = unsafe { ffi::duckdb_data_chunk_get_vector(out_ptr, col_idx as ffi::idx_t) };
    unsafe { ffi::duckdb_vector_reference_vector(dst_vec, src_vec) };
}
```

From src/query/table_function.rs (build_execution_sql, lines 282-322):
```rust
fn build_execution_sql(
    expanded_sql: &str,
    column_names: &[String],
    column_type_ids: &[u32],
) -> String {
    // Currently casts: BIGINT, UBIGINT, STRUCT, MAP, INVALID(VARCHAR)
    // Does NOT cast: DECIMAL, DATE, TIMESTAMP, FLOAT, DOUBLE, BOOLEAN, ENUM, LIST, etc.
}
```

From src/query/table_function.rs (normalize_type_id, lines 197-206):
```rust
pub(crate) fn normalize_type_id(t: u32) -> u32 {
    // Currently normalizes: HUGEINT->BIGINT, UHUGEINT->UBIGINT
}
```

From libduckdb_sys FFI (available for runtime type checking):
```rust
ffi::duckdb_vector_get_column_type(vector: duckdb_vector) -> duckdb_logical_type
ffi::duckdb_get_type_id(type_: duckdb_logical_type) -> duckdb_type
ffi::duckdb_destroy_logical_type(type_: *mut duckdb_logical_type)
```
</interfaces>

<tasks>

<task type="auto">
  <name>Task 1: Fix crash root cause based on Plan 01 diagnosis</name>
  <files>src/query/table_function.rs</files>
  <action>
Apply a fix based on the root cause identified in Plan 01. The fix strategy depends on which crash vector was confirmed.

**If CV-1 (type mismatch in duckdb_vector_reference_vector) -- MOST LIKELY:**

Modify `SemanticViewVTab::func()` in `src/query/table_function.rs` to add a runtime type check before calling `duckdb_vector_reference_vector`. In the zero-copy loop (lines 592-598), BEFORE calling `duckdb_vector_reference_vector(dst_vec, src_vec)`:

1. Get the logical type of both source and destination vectors using `ffi::duckdb_vector_get_column_type()`
2. Extract the type ID from each using `ffi::duckdb_get_type_id()`
3. If types match: proceed with zero-copy `duckdb_vector_reference_vector` (fast path)
4. If types do NOT match: fall back to a safe copy approach -- for now, return an error with a descriptive message identifying the column index, source type, and destination type. This is a diagnostic step; if the type mismatch is common, expand `build_execution_sql` to cast additional types.
5. Destroy both logical types with `ffi::duckdb_destroy_logical_type()` to avoid leaks

Additionally, expand `build_execution_sql` to cast ALL columns (not just BIGINT/UBIGINT/STRUCT/MAP/INVALID). The safest approach: always wrap the expanded SQL in a SELECT with explicit casts for every column matching the bind-time declared type. This eliminates the type mismatch at the source.

Specifically, add these additional type mappings to the cast wrapper:
- DECIMAL -> `DECIMAL` (preserves precision/scale from bind-time declaration)
- FLOAT -> `FLOAT`
- DOUBLE -> `DOUBLE`
- DATE -> `DATE`
- TIMESTAMP -> `TIMESTAMP`
- BOOLEAN -> `BOOLEAN`
- Any unknown type -> wrap in `CAST(col AS VARCHAR)` as last resort

The `needs_wrapping` check should ALWAYS return true (always generate the wrapper SELECT) to ensure type safety. This has negligible performance overhead since DuckDB optimizes away no-op casts.

**If CV-2 (connection lifetime):**
Add `duckdb_disconnect` calls for `query_conn`, `persist_conn`, and `ddl_conn` via a cleanup mechanism. The simplest approach: register a delete callback via `duckdb_table_function_set_extra_info` that calls `duckdb_disconnect` on the raw connection pointer.

**If CV-3 (bind-time SQL):**
Defer the LIMIT 0 type inference from `bind()` to the first call of `func()`. Store a flag in InitData indicating whether schema inference has been done. On first `func()` call, run LIMIT 0 and re-bind columns if needed.

**If CV-4 (materialized vs streaming):**
Switch from `duckdb_result_chunk_count`/`duckdb_result_get_chunk` to `duckdb_fetch_chunk` (streaming API). This is the DuckDB-recommended approach and eliminates the materialized-vs-streaming confusion.

**If multiple crash vectors confirmed:** Fix them in priority order (CV-1 first, then CV-2, etc.).

IMPORTANT: After applying the fix, run `cargo test` to verify the Rust tests still pass. The fix must not break existing functionality.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test 2>&1 | tail -5</automated>
  </verify>
  <done>Root cause fix applied. `cargo test` passes. The specific crash vector identified in Plan 01 is addressed with either a runtime type check, expanded cast wrapper, or connection lifetime fix.</done>
</task>

<task type="auto">
  <name>Task 2: Verify crash fix and full regression suite</name>
  <files>test/integration/test_vtab_crash.py</files>
  <action>
Run the full verification sequence to confirm the fix resolves the crash and introduces no regressions.

1. Build the extension: `just build`
2. Run the crash reproduction script: `python3 test/integration/test_vtab_crash.py`
   - ALL tests must now PASS (no CRASH, FAIL, or TIMEOUT results)
   - If any test still fails, iterate on the fix in Task 1
3. Run the full test suite: `just test-all`
   - `cargo test` must pass (Rust unit + property tests)
   - `just test-sql` must pass (sqllogictest integration)
   - `just test-ducklake-ci` must pass (DuckLake integration)

If the crash reproduction script needs minor adjustments (e.g., a test was testing for a crash that was actually expected behavior, or a timeout needs adjustment), update `test/integration/test_vtab_crash.py` accordingly. Do NOT remove tests -- only adjust expectations if the test was incorrect.

If `just test-all` reveals regressions from the fix, go back to Task 1 and refine the fix. The phase is not complete until ALL tests pass.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && just build && python3 test/integration/test_vtab_crash.py && just test-all 2>&1 | tail -20</automated>
  </verify>
  <done>All crash reproduction tests pass. `just test-all` passes with zero failures. The crash is resolved and no regressions exist.</done>
</task>

</tasks>

<verification>
- `python3 test/integration/test_vtab_crash.py` exits with code 0 (all tests pass)
- `cargo test` passes (Rust unit + property + doc tests)
- `just test-sql` passes (sqllogictest integration tests, requires `just build` first)
- `just test-ducklake-ci` passes (DuckLake Python integration tests)
- `just test-all` passes (combined suite)
</verification>

<success_criteria>
- The crash identified in Plan 01 no longer occurs
- All crash reproduction tests pass
- Full test suite (`just test-all`) passes with zero regressions
- Fix is minimal and targeted (not a rewrite of the query pipeline)
</success_criteria>

<output>
After completion, create `.planning/phases/17.1-python-vtab-crash-investigation/17.1-02-SUMMARY.md`
</output>
