Repository navigation
Conversation
…nt time b6649b7 checked for a concurrent ALTER TABLE ADD/DROP in LocalStorage::commit, which runs after the transaction's records and commit record are already in the WAL. A commit it rejected was rolled back in memory but replayed on the next open, with the zero-filled column it was meant to prevent. An ALTER could also reshape the table between the check and the append, an insert followed by the transaction's own DROP COLUMN failed at COMMIT, and an insert into a renamed table failed once the old name's tombstone came first in the catalog map. The conflict now fails at statement time instead. The transaction manager keeps, per table, the transactions with inserts staged in a local table. A transaction's first insert into a table fails if the newest catalog version, committed or not, no longer exists or has different column IDs or types than its snapshot. ALTER TABLE ADD/DROP COLUMN and DROP TABLE fail while another transaction has staged inserts. The DDL changes the catalog before it checks and both checks run under one mutex, so one side always sees the other. The newest catalog version is read under the catalog set's lock, which catalog rollback now takes too, so a rolled-back ALTER cannot free the entry while it is read.
…bugDB#1221 Committing rows staged before the transaction's own DROP COLUMN still fails a runtime check in NodeGroupCollection::append (LadybugDB#1221), so the case cannot pass until that is fixed.
Contributor
|
4 followups (non-blocking):
|
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
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.
Description
Follow-up to #1435. Its DDL-vs-insert check ran in
LocalStorage::commit, which runs afterwriteCommitToWALhas already logged the transaction and its commit record. A commit it rejected was rolled back in memory but replayed on the next open: with the repro from #1435, reopening without a checkpoint brings back2|20|0, the zero-filled column the check was meant to stop. It also had three other problems:ALTERcould still reshape the table between the check and the append.DROP COLUMNwas rejected atCOMMITas a conflict with another transaction, sincedropColumnbumped the schema epoch without refreshing the transaction's own snapshot. ThatCOMMITno longer reports a conflict, but it still fails a runtime check (and writes the wrong values in a release build) until Bug: DROP in a transaction and ADD after a rolled-back ADD write node column values into the wrong column #1221 is fixed, so there's no test for it here.ALTER TABLE A RENAME TO B, inserts intoBcould fail with the conflict error, becausegetHeadEntryOfOIDreturned whichever head came first in the catalog map, including the old name's tombstone.This replaces the commit-time check with conflicts at statement time, so a rejected insert never reaches the WAL. The transaction manager keeps, per table, the transactions that have inserts staged in a local table. A transaction's first insert into a table fails if the newest catalog version (committed or not) no longer exists or has different column IDs or types than its snapshot. Column IDs rather than names are compared, so a concurrent rename or comment doesn't conflict, but
DROP bfollowed byADD bdoes.ALTER TABLE ... ADD/DROPandDROP TABLEfail while another transaction has staged inserts into the table. The DDL changes the catalog before it checks the registry, and both checks run under one mutex, so whichever comes second sees the other.That changes which side loses. With #1435, an
ALTERalways went through and the inserting transaction failed atCOMMIT. Now the second statement fails: anALTERorDROP TABLEis refused while another transaction holds uncommitted inserts into that table.DROP TABLEbeside staged inserts had no check before #1435. Without one, the inserting commit goes through and the WAL then fails to replay.The insert-side check reads the newest catalog version under the catalog set's lock. Catalog rollback now takes that lock too, so a rolled-back
ALTERcan't free the entry while it's being read.ALTER TABLE ... DROP FROM A TO Bagainst staged rel inserts isn't covered; neither version of the check handled it.Types of changes
Checklist
Not needed: the on-disk format is unchanged.