datastore: add clear_table and fix drop_table
#3214
Conversation
I've fixed the replay parts, but we cannot integration test this until the add-column PR as simply dropping a table is not allowed. |
Co-authored-by: Shubham Mishra <[email protected]> Signed-off-by: Mazdak Farrokhzad <[email protected]>
Centril
force-pushed
the
centril/clear-table-and-fix-drop-table
branch
from
08b70cf to
813f34c
Compare
| let (tx_table, tx_blob_store, delete_table) = self | ||
| .tx_state | ||
| .get_table_and_blob_store_or_create_from(table_id, commit_table); | ||
| let mut rows_removed = tx_table.clear(tx_blob_store); |
There was a problem hiding this comment.
For tx_table, shouldn’t it be enough to just remove it from the BTreeMap of TxState::insert_tables?
There was a problem hiding this comment.
If it wasn't for the blob store you could do that. We could as an optimization check whether the table layout is fixed, in which case there cannot be any blob store references, making the removal sound.
There was a problem hiding this comment.
I think, I don’t fully understand how we’re using blob_store, but for TxState::blob_store, won’t MutTxId::commit eventually drop it anyway?
Do you think is it a problem to occupy extra bytes in blob_store during the lifecycle of tx (until it commits or rollback)?
Description of Changes
Aternative to and closes #3210.
This version relies on
pending_schema_changes.The first commit adds
clear_tableto the datastore that's efficient and can be exposed to the module ABI in a follow up.The second commit fixes
drop_table.API and ABI breaking changes
None
Expected complexity level and risk
3?
Testing
test_drop_table_is_transactionalis amended to checkTxData.