Runs the address reap's writes on SQLite - #144
Merged
Merged
Conversation
StatifierRouter.Addresses.reap/3 stamped and deleted its rows with
fragment("? = ANY(?)", a.id, ^ids), which is Postgres's own form; on
SQLite every reap failed with "no such function: ANY" from 0.8.0 on.
The private stamp/3 and delete/2 now name their rows in an IN list,
fragment("? IN (?)", a.id, splice(^batch)), one bound parameter per id,
which both adapters take. Each id is still bound uncast, so a text key
of digits matches as before. A list of more than 500 ids is written in
batches of 500, one statement each, under SQLite's smallest limit on
bound parameters; a reap answers the same counts either way. The
Postgres reap tests pass unchanged.
Adds StatifierRouter.SQLiteReapTest, which reaps on SQLite under the
default integer key, under a text key of digits and past one batch,
with a test-only store that answers execution statuses from a map.
ADR-0002 gets a dated foot Note; a changelog fragment records the fix.
Refs: sr-1rgb
StatifierRouter.SQLiteMigrationsTest and StatifierRouter.SQLiteReapTest both start StatifierRouter.SQLiteRepo under its module name, and as two async modules they could start it at once: the second start failed with already_started. Both now sit in the :sqlite_repo ExUnit group, whose modules never run concurrently. Refs: sr-1rgb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
StatifierRouter.Addresses.reap/3on SQLite. Refs sr-1rgb.The bug
The private
stamp/3anddelete/2ofStatifierRouter.Addressesnamed their rows withfragment("? = ANY(?)", a.id, ^ids), Postgres's own form, since 0.8.0's:primary_keyoption. On SQLite the statement fails with "no such function: ANY", so every reap of a SQLite host that finds a row to stamp or delete fails. The new tests reproduce it on the base commit, under both key types.The fix
Both functions now use
fragment("? IN (?)", a.id, splice(^batch)): anINlist with one bound parameter per id, which Postgres and SQLite both take. Each id is still bound as the table handed it over, never cast, so the ruleStatifierRouter.Schema.Iddocuments holds as before: a text id made of digits stays a string.The batch bound: a list of more than 500 ids is written in batches of 500, one statement each (the private
in_batches/2,@ids_per_statement). 500 ids plus the stamp's time stays under SQLite's smallest limit on bound parameters (999, the default before SQLite 3.32) and far under Postgres's 65535, so a:limitabove the default cannot build a statement either adapter refuses. TheANYform bound one array whatever the count; a singleINlist without batches would have made a large:limitfail on Postgres too. The countsreap/3answers are the sums over the batches, the same numbers as before, and no ids still sends no statement. At the default:limitof 1000, a reap that stamps or deletes more than 500 rows now sends two statements for that write where it sent one.Postgres answers are unchanged: the existing reap tests in
StatifierRouter.CreateModesTestandStatifierRouter.PrimaryKeyTestpass unchanged; neither file is in this diff.Tests
test/statifier_router/sqlite_reap_test.exs,StatifierRouter.SQLiteReapTest, on the SQLite test repo: a reap that stamps two delivered parcels' rows and deletes an orphan's and one stamped a day ago, then deletes the two it stamped an hour on, under the default integer key and under a:primary_keytext key whose ids are digits with a leading zero; and a reap over 1201 rows, past two batches.test/support/status_store.ex,StatifierRouter.StatusStore: a storage adapter answeringfetch_execution/2from a map, so the SQLite repo needs no execution table.StatifierRouter.SQLiteReapTestand the existingStatifierRouter.SQLiteMigrationsTestboth startStatifierRouter.SQLiteRepounder its module name, so both are in the:sqlite_repoExUnit group, whose modules never run at once. The first CI run of this branch failed on exactly that race (already_started); without the group a local--repeat-until-failure 100run of the two modules failed on its first run, and with it 100 runs passed.Sabotage, one row per check, each restored byte-equal from a copy before the next:
addresses.exstamp/3anddelete/2back on? = ANY(?)in_batches/2answering the last batch's count alonedelete/2onNOT INCreateModesTestreap tests red on PostgresRecords and changelog
:primary_keyAmendment saysstamp/3anddelete/2bind the ids with= ANY. The new Note says what they bind with now and that the Amendment's rule, that the package never casts an id it binds, holds. It decides nothing and removes no line (git diff origin/main -- docs/adr/is additions only).changelog.d/sr-1rgb.mdunder### Fixed.Provenance
Two engineering choices: an
INlist withsplice/1over a per-idORof? = ?comparisons (one flat list rather than a nested expression SQLite's depth limit would bound), and a batch of 500 ids, for the limit above. No dependency changes:mix.exsandmix.lockare not in this diff.The group line in
test/statifier_router/sqlite_migrations_test.exsis a change to a test module this bead did not otherwise touch, forced by the new module sharing its repo.Gate
Full
mix qualityon this head: Format, Compile, Isolated tests, Doc links, Dependencies, Credo, Docs, Tests (425 of 425 passed, 97.7% coverage) and Dialyzer all passed; Doctor, Gettext and Sobelow skipped as not installed.