Let the identity column allocate userid, instead of computing it
`app_users.userid` is a `GENERATED BY DEFAULT AS IDENTITY` column. It did not look like one: `information_schema.columns.column_default` is empty for identity columns, and reading that as "no default at all" is how this came to compute its own id with MAX+1. That worked, and quietly did the wrong thing. An explicit id does not advance the sequence, so two allocators ended up running in parallel — the sequence sat at 1447 while MAX(userid) had reached 9189. They cannot collide today, because almost nothing occupies the range between, but they converge on every insert and the first collision would be a primary key violation on a live sign-up. The insert now omits userid and reads it back with RETURNING. The advisory lock stays, because it was never about the id: two supervisors adding staff at the same instant could both find a PIN free and both take it, and a duplicate PIN attributes a bill to whichever row is read first. Also documents why the email columns go through NULLIF. `app_users_email_unique` is real, and a second cashier created without an email would otherwise collide on the empty string — while NULLs do not collide in Postgres. A cashier who signs in by PIN alone has no email, which is the common case rather than the edge one. Verified in a rolled-back transaction against live data: creation allocates 1448, the sequence advances 1447 -> 1448, a second emailless user is accepted, and a duplicate PIN is still refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -65,11 +65,18 @@ func (r *posRepository) CreatePosUser(tenantID, locationID, configID int, req mo
|
|||||||
var created *models.PosUser
|
var created *models.PosUser
|
||||||
|
|
||||||
err = r.db.Transaction(func(tx *gorm.DB) error {
|
err = r.db.Transaction(func(tx *gorm.DB) error {
|
||||||
// `app_users` has no sequence and no identity — every id in it was
|
// The advisory lock is for the PIN check below, not for the id.
|
||||||
// assigned by hand. So the next one is read and written inside one
|
//
|
||||||
// transaction, behind an advisory lock, or two supervisors creating
|
// `userid` is an identity column — `information_schema.column_default`
|
||||||
// staff at the same moment would compute the same id and one insert
|
// is empty for those, which is easy to misread as "no default at all"
|
||||||
// would lose.
|
// and was misread here once. Postgres allocates it, and this must not
|
||||||
|
// compute its own: an explicit id does not advance the sequence, so a
|
||||||
|
// hand-rolled MAX+1 leaves two allocators running in parallel that
|
||||||
|
// eventually land on the same number.
|
||||||
|
//
|
||||||
|
// The lock still earns its place. Two supervisors adding staff at the
|
||||||
|
// same instant could otherwise both find a PIN free and both take it,
|
||||||
|
// and a duplicate PIN attributes a bill to whichever row is read first.
|
||||||
if err := tx.Exec(`SELECT pg_advisory_xact_lock(hashtext('app_users'))`).Error; err != nil {
|
if err := tx.Exec(`SELECT pg_advisory_xact_lock(hashtext('app_users'))`).Error; err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -95,21 +102,29 @@ func (r *posRepository) CreatePosUser(tenantID, locationID, configID int, req mo
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// `userid` is omitted so the identity column allocates it, and read back
|
||||||
|
// with RETURNING rather than guessed.
|
||||||
|
//
|
||||||
|
// The email columns go through NULLIF because `app_users_email_unique`
|
||||||
|
// is a real constraint: a second person created without an email would
|
||||||
|
// collide on the empty string, while NULLs do not collide in Postgres.
|
||||||
|
// A cashier who signs in by PIN alone has no email, and that is the
|
||||||
|
// common case.
|
||||||
var nextID int
|
var nextID int
|
||||||
if err := tx.Raw(`SELECT COALESCE(MAX(userid), 0) + 1 FROM app_users`).Scan(&nextID).Error; err != nil {
|
if err := tx.Raw(`
|
||||||
|
INSERT INTO app_users
|
||||||
|
(firstname, lastname, authname, email, contactno, password,
|
||||||
|
pin, roleid, configid, tenantid, locationid, status)
|
||||||
|
VALUES (?, ?, NULLIF(?, ''), NULLIF(?, ''), NULLIF(?, ''), NULLIF(?, ''),
|
||||||
|
NULLIF(?, 0), ?, ?, ?, ?, 'Active')
|
||||||
|
RETURNING userid`,
|
||||||
|
first, last, authname, authname, strings.TrimSpace(req.Contactno),
|
||||||
|
password, pin, roleID, configID, tenantID, locationID,
|
||||||
|
).Scan(&nextID).Error; err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
if nextID <= 0 {
|
||||||
if err := tx.Exec(`
|
return fmt.Errorf("the account was not created")
|
||||||
INSERT INTO app_users
|
|
||||||
(userid, firstname, lastname, authname, email, contactno, password,
|
|
||||||
pin, roleid, configid, tenantid, locationid, status)
|
|
||||||
VALUES (?, ?, ?, NULLIF(?, ''), NULLIF(?, ''), NULLIF(?, ''), NULLIF(?, ''),
|
|
||||||
NULLIF(?, 0), ?, ?, ?, ?, 'Active')`,
|
|
||||||
nextID, first, last, authname, authname, strings.TrimSpace(req.Contactno),
|
|
||||||
password, pin, roleID, configID, tenantID, locationID,
|
|
||||||
).Error; err != nil {
|
|
||||||
return err
|
|
||||||
}
|
}
|
||||||
|
|
||||||
created = &models.PosUser{
|
created = &models.PosUser{
|
||||||
|
|||||||
Reference in New Issue
Block a user