[Attach] Fix bug causing sequences to break attaching databases. #11327
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.
This PR fixes #10263
Essentially, this issue stems from code being shared between database loading and attaching a new database.
This function is
SingleFileCheckpointReader::LoadFromStorage
It was originally made to handle loading the initial database from disk, meaning no existing transaction manager was present.
Because of this, it always created a new connection to the database and manually started a transaction, to meet the requirement imposed by the binder - which is that there has to be an active transaction attached to these actions.
Since we are coming from PhysicalAttach, we already have an existing database, and connection with transaction running, in which we are creating this AttachedDatabase.
Because of the behavior of LoadFromStorage, we create a new transaction in which we are loading the contents of the database.
Because there are now two transactions active, one that has created the AttachedDatabase and another that is populating the database, neither transaction can see the changes made by the other.
That causes the lookup for
db1.seq
to fail, because it can't seedb1
.We fix this by providing proper context to LoadFromStorage to let it know there is an existing transaction it can use, instead of starting a new one.