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

Rework find_and_delete_entries (bugfix and better testing) #172

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

hemidactylus
Copy link
Collaborator

@hemidactylus hemidactylus commented Oct 2, 2024

This PR vastly improves the quality, reliability and behaviour of the find_and_delete_entries in the metadata mixin.

  • The batch_size parameter was effectively ignored. FIX: it is no more
  • The testing was scarce. FIX: now various call patterns are tested, with cross-checks by counting the items in the table before-and-after

But most importantly, a major bug has been fixed:
The find_and_delete_entries did not work when in combination with the clustered mixin and/or the multiple primary key mechanism. The reason was that the collection of primary keys (to be later used in deleting) did use the query results after the normalization, so that the flat structure of the table had been made into e.g. {"partition_id": ("x", 10), "row_id": ("a", 1), ...} in case of multicolumn (primary/clustering) key. The deletions then failed miserably.

  • Now the find_entries is split into a pre- and a post-row-normalization parts, so that the find_and_delete_entries can use the former and get access to the actual columns, which is what it needs for the later delete statements.
  • Also comprehensive tests for this column schema have been added.

As a final note, everything in this PR is doubled in its asyncio counterpart.

@hemidactylus hemidactylus requested a review from epinzur October 2, 2024 13:03
@hemidactylus hemidactylus merged commit 3b8aee8 into main Oct 2, 2024
4 checks passed
@hemidactylus hemidactylus deleted the SL-rework-find-and-delete branch October 2, 2024 15:04
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.

2 participants