Partitioned INSERT + write-path expansion (stats, footer-size, partition layout)

[zfarrell]

Context

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

Upstream has a basic src/insert_exec.rs and src/table_writer.rs. The fork expanded these substantially: insert_exec.rs +965 lines, table_writer.rs +1875 lines. The expansion covers partitioned INSERT (Hive layout: partition_col=value/...), column-stats collection during write, multi-file INSERTs, footer-size capture for read-side optimization, encryption-feature gating, and the UploadCleanupGuard orphan-cleanup pattern.

Reference branch

ducklake-features/integration:

• src/insert_exec.rs — physical exec, expanded
• src/table_writer.rs — shared write helpers, substantially expanded
• Tests: tests/write_partition_tests.rs (400 lines, partitioned INSERT round-trips with filesystem inspection of region=US/region=EU/ layout), tests/write_tests.rs (+324 lines), tests/stats_tests.rs (447 lines, column-stats round-trips)

Scope

1. Port the partitioned INSERT path. Partition columns identified from the table's catalog metadata; each output batch routed to the right partition_col=value/ directory; partition values URL-encoded as DuckLake spec requires.
2. Port column-stats collection. During Parquet write, capture Statistics (min/max/null_count, distinct_count where cheap) and register them in ducklake_column_stats (or wherever the spec puts them — verify against current upstream schema after Foundation: rebase integration onto upstream, drop pass-throughs, triage SLT failures #12 rebase).
3. Port footer-size capture and persistence. The fork stores Parquet footer size in metadata for read-side with_metadata_size_hint() optimization; this is already exploited on the read path. Ensure the write path persists it.
4. Port the UploadCleanupGuard pattern — uploaded files are cleaned up on commit failure (already partially mentioned in DELETE physical execution (MOR delete files) #17/UPDATE physical execution (MOR delete + insert) #18 reference, but the source-of-truth implementation lives here).
5. Encryption gating: writes honor the encryption feature flag the same way reads do. Do not break the build when the flag is off.

Acceptance criteria

• tests/write_partition_tests.rs passes — verify region=US/, region=EU/ Hive directories actually contain the right Parquet files with the right rows
• tests/stats_tests.rs passes — TableProvider::statistics() returns correct min/max/null_count after multi-append (the test uses Precision::Inexact(ScalarValue::Int32(10)) style assertions)
• Partition values containing special characters (/, =, spaces, unicode) are correctly URL-encoded
• Concurrent INSERT into the same partition from two transactions: both commit successfully (additive)
• Commit failure mid-INSERT: no orphan files left on disk (verify filesystem inspection in test)
• No duckdb crate imports
• Footer-size is captured during write and read back during scan, eliminating the second-read on Parquet open

Dependencies

• Depends on: Foundation: rebase integration onto upstream, drop pass-throughs, triage SLT failures #12 (foundation)
• Related: DELETE physical execution (MOR delete files) #17 (DELETE), UPDATE physical execution (MOR delete + insert) #18 (UPDATE), MERGE physical execution (INSERT/UPDATE/DELETE atomic) #19 (MERGE) — these share the write-path helpers in table_writer.rs; coordinate to avoid duplicate ports

Out of scope

• Bucketing / hash-partitioning beyond Hive value-partitioning — DuckLake spec doesn't define this
• Adaptive file-size targeting (the compaction question) — separate concern

Notes

• Branch commit references: R11-S-002 (cleanup on partitioned INSERT failure), R10-S-007 (transaction-wrapped append).
• The test parity_basic_crud_after_insert fails on a decimal-formatter mismatch with DuckDB; that's tracked under Foundation: rebase integration onto upstream, drop pass-throughs, triage SLT failures #12, not here.

[zfarrell]

#25 partitioned INSERT + write-path expansion — done

Branch: feat/25-write-path (3 commits on upstream-port/integration)

Cross-engine footer-length resolution

Baseline (foundation, no fix): 34 footer-length failures (the bucket called out in #12's acceptance).
Post-fix: 0.

Root cause: calculate_footer_size_from_bytes returned metadata_len + 8. The DuckLake spec / DuckDB extension stores footer_size as the on-file thrift metadata length only (the i32 LE value at offset file_size - 8), excluding the trailing 4-byte length + 4-byte PAR1. Verified by running the DuckDB DuckLake extension itself: write a 3-row table, read ducklake_data_file.footer_size from the catalog, and compare against the bytes — both equal exactly metadata_len (e.g. 206 for a small file). The R8-S-003 +8 adjustment was incorrect; DuckDB's reader validates footer_length stored in file == footer_size provided and rejected our +8 value on parquet 58 with the seen error.

Fix is one line in src/table_writer.rs::calculate_footer_size_from_bytes. The parquet-rs read-side with_metadata_size_hint() treats the value as a prefetch_hint — underestimating is safe (just triggers an extra ~8-byte fetch), so the lower value is correct on both sides. Footer-size capture remains in place — not deferred.

Acceptance checklist

• 34 cross-engine parquet-footer failures resolved (delta from foundation baseline: 44 non-SLT failures → 12; the 32-test reduction is exactly the footer bucket plus one SLT test slt_insert_insert_column_list; the remaining 12 are unrelated pre-existing baseline failures: float formatting, ALTER PARTITIONED BY plan parse, schema evolution, etc.)
• tests/write_partition_tests.rs passes (8/8 — including 2 new tests added for Partitioned INSERT + write-path expansion (stats, footer-size, partition layout) #25)
• tests/stats_tests.rs passes (7/7)
• Partition values URL-encoded — extended unit test test_build_hive_dir_url_encodes_special_chars to cover spaces (%20), percent literal (%25), Latin-1 accented unicode (café → caf%C3%A9), and CJK multi-byte (日本 → %E6%97%A5%E6%9C%AC). Encoding set in src/insert_exec.rs::url_encode_partition_value covers /, =, %, #, ?, \\, \", <, >, {, }, |, ^, \ , [, ], , plus controls.
• Concurrent INSERT into same partition: both commit — new test test_concurrent_insert_same_partition in write_partition_tests.rs runs two tokio::join!'d INSERTs into region=US and asserts (a) both collect().await succeed, (b) 4 rows are visible after, (c) ≥2 Parquet files exist on disk in main/sales/region=US/.
• Commit failure mid-INSERT: no orphan files — new test test_partitioned_insert_commit_failure_no_orphans uploads 3 files into region=US/ and region=EU/, invokes the exact cleanup_orphaned_files call site insert_exec.rs hits on commit failure, then recursively walks the data dir and asserts zero Parquet files remain. Also confirms the catalog has no registered tables.
• No duckdb crate imports added (verified git diff upstream-port/integration..HEAD)
• Footer-size captured during write, consumed on read — kept; corrected to spec semantics
• cargo build --no-default-features --features write-sqlite clean
• DML chain regression check passes — delete_tests 6/6, update_tests 6/6, merge_tests 5/5

Test surface delta

Metric	Foundation (#12 baseline)	This branch
Total passed	1,248	1,461
Total failed	44	12
Cross-engine footer-length failures	34	0
SLT failed	96	96 (±1 flaky)
Tests ignored	24	24

The +213 passing-count jump comes from the 32 cross-engine tests no longer failing plus tests previously short-circuiting on the footer panic now running to completion (delete/update/merge cross-engine paths exercise downstream assertions).

Footer-size hint status

Preserved. The catalog now stores the correct DuckLake-spec value (metadata_len), and reads via with_metadata_size_hint() continue to work — parquet-rs treats the hint as an under-bounded prefetch.

Commits

5fa9c93 test(writer): concurrent partitioned INSERT and commit-failure cleanup
652cb83 test(insert): cover space, percent, and unicode in partition URL encoding
e403657 fix(writer): correct footer-size capture for DuckLake spec semantics

Out-of-scope failures observed (not opened as new tickets — already tracked)

The 12 remaining non-SLT failures are all pre-existing baseline issues, none of them in #25's scope:

• cross_engine_view_with_aggregation, test_type_decimal_without_parens: float/decimal formatting (Foundation: rebase integration onto upstream, drop pass-throughs, triage SLT failures #12 follow-up territory)
• cross_engine_duckdb_insert_df_delete_duckdb_read: DF-read-after-DF-delete snapshot visibility (DML chain — DELETE physical execution (MOR delete files) #17/UPDATE physical execution (MOR delete + insert) #18)
• test_duckdb_*partition* family (6 tests): DuckDB-native partition transforms (year/month) + ALTER PARTITIONED BY + INSERT-VALUES parse — partition read parity, not write
• test_duckdb_flush_inlined_data, test_duckdb_inlined_data_with_nulls: data inlining round-trip with DuckDB
• test_append_remove_column: schema evolution

Happy to spin these into a separate triage ticket if useful, but each is its own concern.