Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete all table records when dropping tables #1033

Merged
merged 8 commits into from
Oct 25, 2021
Merged

Conversation

chuan
Copy link
Contributor

@chuan chuan commented Oct 23, 2021

Delete the table records when dropping tables. The server side index logic needs to be updated due to table record and catalog info are updated in the same txn. I also moved the reset method in gaia_ptr from protected to public.

Copy link
Contributor

@senderista senderista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just flagged a few things to discuss.

production/catalog/src/ddl_executor.cpp Outdated Show resolved Hide resolved
production/catalog/src/ddl_executor.cpp Outdated Show resolved Hide resolved
production/catalog/src/ddl_executor.cpp Outdated Show resolved Hide resolved
production/db/core/src/index_builder.cpp Outdated Show resolved Hide resolved
production/inc/gaia_internal/db/gaia_ptr.hpp Show resolved Hide resolved
production/inc/gaia_internal/db/gaia_ptr.hpp Show resolved Hide resolved

begin_transaction();
gaia::addr_book::customer_t::delete_row(customer_id);
EXPECT_EQ(gaia::addr_book::customer_t::list().size(), 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this call fail? Maybe not as part of this PR, and maybe it makes sense it does not fail, just worth thinking about it. If we think this should fail you can add a follow up jira.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created GAIAPLAT-1623 and added some comments.

Copy link
Contributor

@senderista senderista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine to merge. We can address the issues raised elsewhere.

@chuan chuan merged commit ab1ecd7 into master Oct 25, 2021
@chuan chuan deleted the chuan/drop-table branch October 25, 2021 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants