docs(etl): correct why the users insert needs ON CONFLICT DO NOTHING - #429
Merged
Conversation
#425 justified the clause with the claim that nothing validates that a user is new. That is wrong: validateUserCreate has called userExists since #148, and migratedUserCreateHandler does the same on the genesis replay path. The clause is still needed, for a different reason. userExists followed by INSERT is check-then-act, and that is atomic only within one transaction. A single writer therefore cannot reach the conflict, but a second writer can, by passing its own check before this insert commits — which is the most plausible account of the five duplicate current rows 0035 cleans up, three of which pair a bare-hex txhash with a 0x-prefixed one. users_current_uniq_idx is what closes that race. DO NOTHING decides what the loser does about it, and no-op is both correct (the row it would write already exists) and better than the alternative: a hard error here rolls back the savepoint and drops the tx along with its audit row, which is silent apart from a log line. Comments only — no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rickyrombo
force-pushed
the
fix/drop-unneeded-user-insert-conflict
branch
from
August 4, 2026 21:28
7d96170 to
994007e
Compare
rickyrombo
changed the title
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
1 participant
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.
Follow-up to #425, correcting a claim I made there. Comments only — no behaviour change.
What was wrong
#425 justified
ON CONFLICT DO NOTHINGon the users insert with "nothing validates that a user is new."That is false.
validateUserCreatehas calleduserExistssince #148, andmigratedUserCreateHandlerdoes the same on the genesis replay path.Why the clause is still right
My first pass at this PR removed the clause as dead code. That was also wrong, and for an interesting reason:
userExists→INSERTis check-then-act, which is atomic only within a transaction.userExistsbefore this insert commits, and both insert. Nothing in the code prevents it.The second case is not hypothetical: it is the most plausible account of the five duplicate current rows
0035cleans up, three of which pair a bare-hextxhashwith a0x-prefixed one.So
users_current_uniq_idxis what actually closes the race.DO NOTHINGonly decides what the loser does about it — and a no-op is both correct (the row it would write already exists) and better than the alternative: a hard error here hitssp.Rollback+return falseinindexer.go, dropping the tx and its audit row, silent apart from a log line. That is the same path that silently lost 30 txs during the genesis replay.Changes
user_create.go— rewrite the comment aboveinsertUserWithState.0035_users_one_current_row.up.sql— rewrite the paragraph asserting the missing check.The constraint, the backfill, and the fifteen genesis-writer joins are all untouched.
🤖 Generated with Claude Code