Fix sequence number check when inserting new events#515
Merged
erikrozendaal merged 1 commit intomasterfrom Feb 26, 2026
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical bug in the store_events procedure where the MIN() aggregate function was performing lexicographic (text) comparison instead of numeric comparison on sequence numbers. This caused false optimistic locking failures when committing batches of 10+ events, as MIN('9', '10') returns '10' lexicographically instead of '9' numerically.
Changes:
- Fixed the MIN() aggregate to cast sequence_number to integer before comparison in store_events_v05.sql and db/structure.sql
- Added a test case that reproduces the bug scenario (committing events with sequence numbers crossing digit boundaries)
- Created a migration to apply the fix to existing databases
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| db/migrate/sequent/store_events_v05.sql | New version of store_events procedure with the MIN() bug fix, changing from text extraction (->>) to JSONB extraction with integer cast (->' ... '::integer) |
| db/structure.sql | Updated to reflect the fixed store_events procedure and added the new migration version |
| db/migrate/20260226121600_sequent_fix_sequence_number_check.rb | Migration to apply the fix, but contains a critical bug in the down method |
| spec/lib/sequent/core/event_store_spec.rb | Added regression test that commits 8 events followed by events 9 and 10 to verify the fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The `MIN` aggregate function was sorting the sequence number using the `text` datatype, which does not correctly find the minium. Cast to `integer` to fix this. See #514.
erikrozendaal
force-pushed
the
514-bug-store_events_v03sql-uses-text-min-on-sequence_number-causing-false-optimistic-locking-failures
branch
from
1269c1b to
77d6f01
Compare
erikrozendaal
deleted the
514-bug-store_events_v03sql-uses-text-min-on-sequence_number-causing-false-optimistic-locking-failures
branch
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.
The
MINaggregate function was sorting the sequence number using thetextdatatype, which does not correctly find the minium. Cast tointegerto fix this.See #514.