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

Use TableDescriptor in InMemoryTable #111

Merged
merged 1 commit into from
Dec 24, 2024
Merged

Conversation

lavrukov
Copy link
Contributor

No description provided.

@lavrukov lavrukov requested a review from g-sg-v December 23, 2024 18:18
@lavrukov lavrukov force-pushed the in-memory-multy-tables branch 2 times, most recently from 7ccfc12 to 39c5b09 Compare December 23, 2024 18:45
var tables = repo.tables();
new StdTxManager(repo).tx(() -> tables
.forEach(table -> Tx.Current.get().getRepositoryTransaction().table((Class) table).deleteAll()));
repository.dropDb();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is it possible to do it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You should probably keep the old logic, which deletes all data from the tables actually created (from repo.tables()), but DOES NOT DROP THE TABLES themselves. You should be able recreate the old logic just fine, because now repo.tables() returns a set of TableDescriptors

@lavrukov lavrukov requested a review from nvamelichev December 23, 2024 18:54
Copy link
Collaborator

@nvamelichev nvamelichev left a comment

Choose a reason for hiding this comment

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

👍 Looks good, see my comment about cleanup vs dropDb()

var tables = repo.tables();
new StdTxManager(repo).tx(() -> tables
.forEach(table -> Tx.Current.get().getRepositoryTransaction().table((Class) table).deleteAll()));
repository.dropDb();
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should probably keep the old logic, which deletes all data from the tables actually created (from repo.tables()), but DOES NOT DROP THE TABLES themselves. You should be able recreate the old logic just fine, because now repo.tables() returns a set of TableDescriptors

@lavrukov lavrukov force-pushed the in-memory-multy-tables branch from 39c5b09 to e290822 Compare December 24, 2024 13:53
Copy link
Contributor

@g-sg-v g-sg-v left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Collaborator

@nvamelichev nvamelichev left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@nvamelichev nvamelichev merged commit 286b5b9 into main Dec 24, 2024
1 check passed
@nvamelichev nvamelichev deleted the in-memory-multy-tables branch December 24, 2024 20:43
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