feat(vortex): write DuckLake data files in the vortex format (INSERT/CTAS) - #2
Merged
Merged
Conversation
…CTAS)
Completes the vortex round trip: INSERT/CTAS into a table whose data_file_format
is 'vortex' now writes real .vortex files and reads them back.
- GetCopyOptions: non-parquet formats use the CHANGED_ROWS_AND_FILE_LIST copy
return type (vortex's C-API COPY can't report per-file statistics) and a
single explicit output file path (vortex doesn't implement rotate_next_file,
so DuckDB's directory+rotation naming would hand it the table dir, not a file)
- AddWrittenFiles: parse {count, files[]} for non-parquet writes and fstat the
file for its size; resolve the recorded format once on DuckLakeInsertGlobalState
so INSERT, CTAS, flush and compaction all record it
- metadata: guard the column-stats INSERT/UPDATE and global-stats readback for
files that carry no column statistics (statless formats); keep such files
through filter pushdown instead of pruning them (they have no min/max to prune
on). These are no-ops for parquet, which always has stats.
- test/sql/vortex/vortex_write.test: INSERT + CTAS round trip incl. types,
NULLs, nested lists, filter pushdown, deletes and updates (49 assertions)
- CI: exclude windows arches (DuckDB v1.5.1 vendored fmt vs current MSVC) and
make the debug build manual-only (from-scratch debug build exceeds the runner
time limit on this fork)
Verified: vortex_write 49, full DuckLake sql suite 91550/91551 (the one failure
is an unrelated ducklake_max_retry_count RESET-default artifact of the DuckDB pin,
not touched by this change).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Bugbot on #2 flagged three cases where a non-parquet (statless) write breaks a DuckLake assumption that files carry column statistics. Reject them with clear errors instead of crashing or silently corrupting: - partitioned writes: need one file per partition value + recorded partition_values, which the single-file non-parquet path cannot provide - flushing inlined data: derives begin_snapshot / row_id_start from the written file's snapshot_id / row_id column stats, absent for non-parquet - NOT NULL columns: enforced by inspecting written null-count stats, absent for non-parquet, so nulls could otherwise slip into a NOT NULL column Extends vortex_write.test to cover the three rejections (58 assertions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
moshap-firebolt
force-pushed
the
firebolt/vortex-write
branch
from
August 5, 2026 00:23
f9f6b11 to
917985f
Compare
Addresses the SQLite/Postgres CI failure and the orphan-file review comment.
- filter pushdown: the previous statless-file fix referenced the stats CTE
twice (IN ... OR NOT IN ...) while it stayed NOT MATERIALIZED, which
double-evaluated the subquery and broke the sqlite/postgres metadata backends
("Attempted to access index 0 within vector of size 0"). Instead, build the
CTE as a LEFT JOIN from ducklake_data_file to its column stats: files with no
stats row appear with NULL stats and are kept by the existing null checks,
with a single CTE reference. Works on all backends.
- move the flush and NOT NULL write guards from AddWrittenFiles (which runs
after PhysicalCopyToFile has already written the file, leaving an orphan on
disk) into GetCopyOptions, alongside the partition guard, so unsupported
non-parquet writes fail before anything is written. NOT NULL is detected via
a new DuckLakeCopyInput.has_not_null_columns flag; flush via the
WRITE_ROW_ID_AND_SNAPSHOT_ID virtual-column marker.
Verified: vortex_write 58, full DuckLake sql suite 91559/91560 (only the
unrelated max_retry_count RESET artifact), rejected inserts leave no orphan
.vortex files.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b5310ed. Configure here.
…deploy Bugbot on #2 noted the flush guard also blocked compaction: both flush and merge_adjacent_files set WRITE_ROW_ID_AND_SNAPSHOT_ID, so keying the guard on virtual_columns rejected vortex compaction with a misleading flush error. - distinguish the two with explicit DuckLakeCopyInput flags (is_flush / is_compaction) set by their respective callers, instead of virtual_columns - flush stays rejected (needs begin_snapshot / row_id_start recovered from the written file's stats). Compaction is also rejected for now, but with its own message: its directory+rotation output model does not compose with the single-file path used for non-parquet writes (it otherwise produced a malformed <file>.vortex/<file>.parquet path). Both fail in GetCopyOptions, before any write, so no orphan files. - CI: exclude windows from the deploy matrix too (it is already excluded from the build), fixing the failed nightly-deploy job. vortex_write.test covers the compaction rejection (62 assertions). Parquet compaction unaffected (compaction suite 1844 assertions pass). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1 (vortex reads). Completes the round trip:
INSERT/CREATE TABLE ASinto a table withdata_file_format='vortex'now writes real.vortexfiles and reads them back.What changed
GetCopyOptions: non-parquet formats use theCHANGED_ROWS_AND_FILE_LISTcopy return type (vortex's C-API COPY can't report per-file statistics) and a single explicit output file path (vortex doesn't implementrotate_next_file, so DuckDB's directory+rotation naming would hand it the table directory instead of a file).AddWrittenFiles: parse{count, files[]}for non-parquet writes andfstatthe file for its size; the recorded format is resolved once onDuckLakeInsertGlobalStateso INSERT, CTAS, flush and compaction all record it.INSERT/UPDATEand the global-stats readback for files that carry no column statistics, and keep such files through filter pushdown instead of pruning them (no min/max to prune on). All no-ops for parquet, which always has stats.Test —
test/sql/vortex/vortex_write.test(49 assertions): INSERT + CTAS round trip incl. types, NULLs, nested lists, filter pushdown, deletes and updates.Verified — vortex_write 49/49; full DuckLake sql suite 91550/91551. The single failure is an unrelated
ducklake_max_retry_countRESET-default artifact of the DuckDB pin (no settings code in this diff).CI — excludes windows arches (DuckDB v1.5.1 vendored fmt vs current MSVC) and makes the debug build manual-only (from-scratch debug build exceeds the runner time limit on this fork).
🤖 Generated with Claude Code
Note
Cursor Bugbot is generating a summary for commit 9f7762b. Configure here.