R10/R11 hardening fixes batch (correctness + atomicity safety net)

[zfarrell]

Context

This is one ticket in a series carrying forward #12 foundation work. Read #12 first for repo context.

The fork went through ten review cycles (R1–R11) and accumulated a substantial set of correctness, atomicity, and overflow-safety fixes. Some are tied to specific feature workstreams and travel with those tickets (#17/#18/#19/#20). The ones that don't have a clear home — generic safety net improvements — live here.

This ticket exists so the safety improvements don't get lost when the feature tickets are extracted into focused PRs.

Reference branch

ducklake-features/integration — search commit log for R10-S-* and R11-S-* prefixes. Read docs/2026-03-03-retrospective-r1-r5.md and docs/2026-03-05-retrospective.md for the design intent.

Specific fixes to port (verify each is still applicable after #12 rebase)

Numbered by their R*-S-* reference in the branch commit log:

• R10-S-007 — wrap append-mode file registration in a transaction
• R10-S-020 — DROP SCHEMA CASCADE atomicity (delegates to drop_schema_inner) — also tracked under ALTER/DROP/CREATE schema evolution DDL #20
• R11-S-002 — clean up uploaded files on partitioned INSERT commit failure (already part of DELETE physical execution (MOR delete files) #17/UPDATE physical execution (MOR delete + insert) #18 patterns; verify it's there)
• R11-S-003 — detect multiple source rows matching same target in MERGE (tracked under MERGE physical execution (INSERT/UPDATE/DELETE atomic) #19; verify)
• R11-S-007 — propagate type-conversion errors for delete_file_id in DuckDB provider
• R11-S-009 — normalize type strings to lowercase in is_type_promotion_allowed (tracked under Type system & inlined-data parsing (merge with upstream overlap) #23)
• R11-S-010 — reject NaN/Inf in compaction delete_threshold (compaction is dropped; the NaN/Inf check pattern may still apply to other float inputs — audit)
• R11-S-012 — include nullability in schema equality check across all backends
• R11-S-013 — checked_mul for timestamp unit conversions (tracked under Type system & inlined-data parsing (merge with upstream overlap) #23)
• R11-S-014 — saturating_add for null count accumulation (tracked under Type system & inlined-data parsing (merge with upstream overlap) #23)
• R11-S-015/017/029 — resolve unstable as_str() and unused import warnings; extract shared Parquet write helper for UPDATE/MERGE; as_ref() instead of unstable as_str() in SQLite inlined data
• R11-S-016 — reuse target key Vec in MERGE hash join loop (perf; tracked under MERGE physical execution (INSERT/UPDATE/DELETE atomic) #19)
• R11-S-018 — snapshot-aware predicate in get_table_structure
• R11-S-019 — i32::try_from for Date32 num_days (tracked under Type system & inlined-data parsing (merge with upstream overlap) #23)
• R11-S-020 — guard decimal scale range to prevent 10i128.pow() overflow (tracked under Type system & inlined-data parsing (merge with upstream overlap) #23)
• R11-S-021 — checked_add for delete_filter row_offset arithmetic
• R11-S-022/025 — additional snapshot-awareness fixes in list_all_columns
• R11-S-026 — tighten SLT DOES NOT EXIST pattern to DETACH only (test infrastructure)
• R11-S-027 — compare_exchange to prevent double-install race (was tied to compaction; pattern may still apply elsewhere)
• R11-S-028 — wrap inlined data reads in transaction to prevent TOCTOU
• R11-S-030 — replace table_id.unwrap() with proper error in next_entity_id
• R11-S-031 — cross-engine test for DF-created partitioned data read by DuckDB
• R11-S-033 — precompute reorder mapping in cdc_common (perf; tracked under CDC table functions (with cdc_common duplicate-column bug fix) #21)

Scope

1. For each fix above, check whether it's already on the rebased branch (most should be) — confirm don't re-do.
2. For each fix that's tied to a specific feature ticket (DELETE physical execution (MOR delete files) #17/UPDATE physical execution (MOR delete + insert) #18/MERGE physical execution (INSERT/UPDATE/DELETE atomic) #19/ALTER/DROP/CREATE schema evolution DDL #20/CDC table functions (with cdc_common duplicate-column bug fix) #21/Type system & inlined-data parsing (merge with upstream overlap) #23), confirm the feature ticket actually pulled it in. If not, this ticket is the safety net.
3. The remaining fixes (R10-S-007, R11-S-007, R11-S-012, R11-S-018, R11-S-021, R11-S-022, R11-S-025, R11-S-028, R11-S-030) don't belong to a single feature ticket — port them here.

Acceptance criteria

• Each fix in the list above is either confirmed present (via test or code grep) or ported in this PR
• No regressions in the active test surface from Foundation: rebase integration onto upstream, drop pass-throughs, triage SLT failures #12
• Snapshot-awareness audit (docs/2026-03-07-snapshot-awareness-audit.md on the branch) findings all addressed in code
• No duckdb crate imports added

Dependencies

• Depends on: Foundation: rebase integration onto upstream, drop pass-throughs, triage SLT failures #12 (foundation)
• Related: every feature ticket — this is the safety net

Out of scope

• Reviewing the design choices behind each fix — they were already reviewed in cycles R1–R11
• Adding new safety improvements that weren't in the R10/R11 cycle — separate effort

Notes

• The fork's review-cycle docs in docs/ describe each finding's rationale. Read the relevant doc before porting any non-obvious fix.
• Several of these are 1-3 line changes; do not over-engineer.

[zfarrell]

#24 Audit: All R10/R11 hardening fixes verified-present

Audited every R-numbered fix from ticket #24's list against the current upstream-port/integration tree. All fixes survived the foundation port — nothing missing, nothing to port here.

Categorization

R-id	Status	Evidence
R10-S-007	verified-present	append_table_files trait method + sqlx impls (metadata_writer.rs:521, metadata_writer_impl.rs:769)
R10-S-020	verified-present	drop_schema_inner macro (metadata_writer_impl.rs:2013); catalog.rs:264 delegates atomic cascade
R11-S-002	verified-present	"R11-S-002" anchor at insert_exec.rs:816; cleanup_orphaned_files at table_writer.rs:1601
R11-S-003	verified-present	"R11-S-003" anchor at merge_exec.rs:491 (multiple-source-rows MERGE violation error)
R11-S-007	verified-present	row.get::<_, Option<i64>>(6)?.is_some() at metadata_provider_duckdb.rs:269,461
R11-S-009	verified-present	to_lowercase() in is_type_promotion_allowed (metadata_writer.rs:88-89)
R11-S-010	not-applicable	Compaction was dropped in the foundation port. NaN-filter pattern is still applied to other float inputs in table_writer.rs stats (1410+) — no audit gaps
R11-S-012	verified-present	*nullable == col.is_nullable in schema_matches across all 3 sqlx writers (metadata_writer_{sqlite,postgres,mysql}.rs:563/446/563)
R11-S-013	verified-present	checked_mul for timestamp unit conversions (insert_exec.rs:436,440; table_writer.rs:1158,1174)
R11-S-014	verified-present	saturating_add for null counts (table.rs:1381, table_writer.rs:1349)
R11-S-015	verified-present	&**name deref instead of unstable as_str() (table_writer.rs:1250,1263); ObjectStore unused-imports removed from merge_exec.rs/update_exec.rs
R11-S-016	verified-present	Reusable target_key: Vec<HashableKeyValue> (merge_exec.rs:453-474)
R11-S-017	verified-present	Shared write_and_upload_parquet helper (table_writer.rs:1672); used by update_exec.rs:422 and merge_exec.rs:622
R11-S-018	verified-present	Snapshot-aware SQL_GET_TABLE_COLUMNS (metadata_provider.rs:27-32); all 4 providers pass (table_id, snapshot_id, snapshot_id)
R11-S-019	verified-present	i32::try_from for Date32 days (parse_values.rs:130)
R11-S-020	verified-present	scale_u > 38 guard against 10i128.pow() overflow (parse_values.rs:365)
R11-S-021	verified-present	checked_add for row_offset (delete_filter.rs:161,194)
R11-S-022	verified-present	Snapshot-aware column predicate in SQL_LIST_ALL_COLUMNS (metadata_provider.rs:297-298) — 6 bindings instead of 4
R11-S-025	verified-present	Same SQL_LIST_ALL_COLUMNS fix; also unimplemented!() for non-atomic default trait impls (metadata_writer.rs:489,508,528)
R11-S-026	verified-present	Tightened to Some("DETACH") (tests/sqllogictest_runner.rs:716)
R11-S-027	not-applicable	Compaction was dropped; no double-install pattern exists elsewhere — no compare_exchange/AtomicBool usage anywhere in the tree
R11-S-028	verified-present	let mut tx = self.pool.begin() wrapping inlined-data reads in all 3 sqlx providers (metadata_provider_{sqlite,postgres,mysql}.rs:205/191/199)
R11-S-029	verified-present	&*col.name() instead of unstable as_str() in SQLite store_inlined_data (metadata_writer_sqlite.rs:1343)
R11-S-030	verified-present	table_id.ok_or_else (metadata_writer_sqlite.rs:708). Postgres/MySQL ignore the _table_id parameter so no analogous unwrap exists there
R11-S-031	verified-present	test_df_insert_partitioned_duckdb_read (tests/cross_engine_partition_tests.rs:282)
R11-S-033	verified-present (reverted-final)	position() scan retained (cdc_common.rs:104) with the explanatory comment from the revert about duplicate projections breaking test_cdc_projection_duplicate_columns

Snapshot-awareness audit cross-check

Read .audit/2026-03-07-snapshot-awareness-audit.md. The two BUGs it identifies — get_table_structure and list_all_columns missing snapshot predicates — are both fixed in the SQL constants (metadata_provider.rs:27-32 and metadata_provider.rs:282-299). All 4 providers pass the matching number of snapshot bindings. No survivors.

Ports performed

None. All fixes were already carried in commit fca0d0d ("chore: foundation port from ducklake-features/integration"). The feature-ticket-tracked fixes (#17/#18/#19/#20/#21/#23/#25) and the generic safety-net fixes all landed together in the squash.

Acceptance criteria

• Each R-numbered fix categorized: verified-present / not-applicable
• No regressions introduced (no code changes made)
• Snapshot-awareness audit findings all addressed in code
• No duckdb crate imports added (no changes made)
• Standalone fixes ported: none required

Test baseline

cargo test --features write-sqlite --no-fail-fast: 1428 passed, 140 failed, 24 ignored on upstream-port/integration. The 140 failures are pre-existing on the foundation port (e.g., "Parquet footer length stored in file is not equal to footer length provided" in cross_engine_alter_tests) and tracked under other tickets; they are unchanged by this verification.

Branch

No branch pushed — there were no changes to commit. Local feat/24-hardening was created, found empty, and deleted.

Finding

The foundation squash port (fca0d0d) was thorough — every safety improvement from R10/R11 was preserved. No work was lost in the rebase as far as this hardening checklist is concerned.