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

row_count field to table #1242

Merged
merged 2 commits into from
May 17, 2024
Merged

row_count field to table #1242

merged 2 commits into from
May 17, 2024

Conversation

Shubham8287
Copy link
Contributor

@Shubham8287 Shubham8287 commented May 16, 2024

Description of Changes

Meta data field in Table struct for row count.

API and ABI breaking changes

N/A

Expected complexity level and risk

*How complicated do you think these changes are? Grade on a scale from 1 to 5,
2

@Shubham8287 Shubham8287 changed the title row_coun field to table row_count field to table May 16, 2024
@Shubham8287 Shubham8287 force-pushed the shub/table-rows branch 2 times, most recently from 3b5b2f0 to 75391ce Compare May 16, 2024 17:35
@Centril Centril self-requested a review May 17, 2024 07:12
@Shubham8287 Shubham8287 self-assigned this May 17, 2024
@gefjon
Copy link
Contributor

gefjon commented May 17, 2024

Please add test(s) which exercise the following paths and assert the row count is correct:

  • Inserts.
  • Inserts followed by deletes.
  • Insert of an already-present row, causing a set-semantic violation.
  • core::db::datastore::locking_tx_datastore::mut_tx::MutTx::delete_by_row.
  • Transaction replay.

@Shubham8287
Copy link
Contributor Author

  • core::db::datastore::locking_tx_datastore::mut_tx::MutTx::delete_by_row.

I didn't get what it means, considering row_count is only implemented for read tx. I may add test if you explain.

Added test for rest of the paths.

@Shubham8287 Shubham8287 added this pull request to the merge queue May 17, 2024
Merged via the queue into master with commit 0c0567e May 17, 2024
6 checks passed
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.

4 participants