From f343f4e86e9bff3438f8e519cbf9746e9b6571e1 Mon Sep 17 00:00:00 2001 From: Suriya Date: Thu, 6 Aug 2026 20:30:18 +0530 Subject: [PATCH] Let the identity column allocate userid, instead of computing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- repositories/posUserRepository.go | 49 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/repositories/posUserRepository.go b/repositories/posUserRepository.go index a00e270..e1f3a99 100644 --- a/repositories/posUserRepository.go +++ b/repositories/posUserRepository.go @@ -65,11 +65,18 @@ func (r *posRepository) CreatePosUser(tenantID, locationID, configID int, req mo var created *models.PosUser err = r.db.Transaction(func(tx *gorm.DB) error { - // `app_users` has no sequence and no identity — every id in it was - // assigned by hand. So the next one is read and written inside one - // transaction, behind an advisory lock, or two supervisors creating - // staff at the same moment would compute the same id and one insert - // would lose. + // The advisory lock is for the PIN check below, not for the id. + // + // `userid` is an identity column — `information_schema.column_default` + // is empty for those, which is easy to misread as "no default at all" + // 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 { 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 - 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 } - - if err := tx.Exec(` - 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 + if nextID <= 0 { + return fmt.Errorf("the account was not created") } created = &models.PosUser{