DuckLake: make CREATE OR REPLACE TABLE atomic; end sort orders on drop - #17
Open
fuziontech wants to merge 3 commits into
Open
DuckLake: make CREATE OR REPLACE TABLE atomic; end sort orders on drop#17fuziontech wants to merge 3 commits into
fuziontech wants to merge 3 commits into
Conversation
Both forms of the statement, the DDL one and the CTAS one, used to fail with "This connector does not support replacing tables". A writer that wanted to rebuild a table had to run DELETE and then INSERT, which is two DuckLake snapshots with an empty table visible in between. Anything reading the table at that moment reads nothing. A replace is now one snapshot, as it is in DuckDB. Within it the table that holds the name is ended -- its row, its columns, its partitioning, its data and delete files, its tags and its statistics, exactly what DROP TABLE ends -- and the table that takes the name is created. A reader at the snapshot before sees the old rows and a reader at the snapshot after sees the new ones. DuckDB does the same, in DuckLakeSchemaEntry::HandleCreateConflict: it drops the existing entry and creates a new one with a fresh catalog id, both staged in one transaction and committed as one snapshot. So the replacement is a new table, with a new identifier, and nothing of the old one carries over. This connector matches that: a comment or a partitioning the new definition does not state is gone with the old table. It differs in one place, which is the data directory. DuckDB derives it from a fresh table UUID, while this connector names it after the table, so a replaced table keeps writing into the directory the old one used. Files there are named by UUID and never collide, and a drop followed by a create of the same name already behaves this way. The CTAS form cannot create its table up front the way the plain form does, because the name still has to resolve to the table being replaced while the rows are written. It carries a DuckLakeReplaceTarget through the write instead, which holds only what the workers need, and finishCreateTable makes the one commit that ends the old table, creates the new one and registers the files. A commit that loses a race is still discarded and replayed against the newer state, as every other commit here is. Views keep working across a replace, because a view refers to the table by name. Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
Test methods of a class run in parallel against one catalog, and several of them write to it from DuckDB while others write to it from Trino. Every change to a DuckLake catalog claims the next snapshot, so two writers that start from the same one conflict. The connector retries such a commit. DuckDB reports it, and the test that ran the statement fails. That was already possible, and it got likelier with five more tests writing to the same catalog: TestDuckLakeWrites failed twice in seven runs, each time in a different test, always on a DuckDB statement. So the fixture now runs such a statement again, which is what a writer facing the conflict would do. A failed commit leaves the connection unusable, so each attempt after the first opens a new one. Options an earlier statement set survive that, because DuckLake keeps them in the catalog rather than on the connection. TestDuckLakeWrites and TestDuckLakeReads ran eight times each without a failure afterwards. Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
DuckDB ends ducklake_sort_info along with the rest of a table it drops, in DuckLakeMetadataManager::DropTables. This connector ended everything else that method ends and left the sort order behind, so a table DuckDB had sorted kept a live sort_info row pointing at a table id that no longer resolved. That is worse than a stale row. DuckDB validates sort entries when it attaches a catalog, so the leftover row made every later DuckDB connection fail with "Invalid Input Error: Could not find matching table for sort entry" -- the catalog could no longer be opened at all. Both tests below reproduce exactly that without the fix. ducklake_sort_info arrived in format version 0.4, and the connector still reads 0.1 catalogs, so the metastore checks for the table the way it already checks for ducklake_view and ducklake_name_mapping, and passes the answer to the commit. The check is cached, and runs outside the commit transaction so a commit still needs one connection. Rows keyed by a sort order rather than by the table, such as ducklake_sort_expression, carry no snapshot range and are left alone, as DuckDB leaves them. Two further things, both about work done for nothing: beginCreateTable now refuses to replace a name a view holds, instead of leaving that to the commit. The commit keeps the authoritative check, which is what catches a view created after this read. But every row the statement selects is written before that commit runs, and none of it could be used, so the cheap read here saves the whole write. TestDuckLakeAbsoluteSchemaPath covers reading a schema whose catalog row holds an absolute path. It needs a catalog of its own, because that path names a Trino file system DuckDB cannot resolve. Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
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.
Summary
CREATE OR REPLACE TABLE, DDL and CTAS forms, is now one DuckLake snapshot instead ofNOT_SUPPORTED. Inside one commit the table holding the name is ended — its row, columns, partitioning, data and delete files, tags, stats and sort order, exactly whatDROP TABLEends — and a new table with a fresh id is created; for CTAS the files written during the query are registered under the new id in the same commit. A reader at snapshot S−1 sees the old rows, at S the new rows, never none. Matches DuckDB'sHandleCreateConflict(new id; nothing of the old definition carries over). One deliberate difference: the data directory is named after the table, so a replaced table keeps writing into the old directory (files are UUID-named and registered per table id).beginCreateTable: that would publish an empty table for the duration of the write. It carries aDuckLakeReplaceTarget(column layout + location) andfinishCreateTablemakes the single commit.beginCreateTable(replace=true)refuses a name held by a view before any file is written.endTableContentsdid not endducklake_sort_info. Dropping (or now replacing) from Trino a table DuckDB created withSET SORTED BYleft a live sort row on a dead table id, and DuckDB then failed everyATTACHof the catalog withCould not find matching table for sort entry. Fixed for bothDROP TABLEand REPLACE, guarded on the table's presence (format ≥ 0.4).DuckLakeCommit.SchemaIdentitycarriespath_is_relative, so a schema with an absolute location resolves correctly on create.ducklake.mdno longer says the connector is read-only; a new SQL support section lists supported statements (incl.OR REPLACE) and Limitations matches theNOT_SUPPORTEDthrows that remain.Tests
TestDuckLakeWrites: replace by DDL, by CTAS (old files ended, not deleted), replace keeps nothing of the old definition, replace of a missing table (DDL, CTAS, zero-row CTAS), a view over a replaced table reads the new rows, view/table name clash refused before writing (asserts no directory appears), sort order ended on replace and on drop (with the fix reverted, both tests fail atATTACH— the corruption above). A shared helper assertsold.end_snapshot == new.begin_snapshot, distinct ids, and the exactchanges_madestring.TestDuckLakeAbsoluteSchemaPath: a table created in a schema with an absolute location lands its files there and reads back.-P cichecks clean; Sphinx builds with-W.Not in this PR
ALTER SCHEMA … RENAME TOhas never worked against a Postgres catalog:ducklake_schema.schema_idis a bare primary key andrenameSchemaends-then-inserts the same id, which fails23505and is masked by the retry loop as "concurrent updates". DuckDB has no schema rename. Needs a decision (in-place UPDATE vs. unsupported); tracked separately.ducklake_snapshotrow last where DuckDB claims it first, so a cross-engine race can surface as a Postgres deadlock rather than a clean conflict. Commit-protocol change; tracked separately.https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe