Skip to content

refactor(db): make SQL layer portable and concurrency-safe#754

Merged
gabriel-samfira merged 1 commit into
cloudbase:mainfrom
cbartz:feat/postgres-groundwork
Jun 1, 2026
Merged

refactor(db): make SQL layer portable and concurrency-safe#754
gabriel-samfira merged 1 commit into
cloudbase:mainfrom
cbartz:feat/postgres-groundwork

Conversation

@cbartz
Copy link
Copy Markdown
Contributor

@cbartz cbartz commented Jun 1, 2026

What

Two backend-agnostic improvements to the GORM SQL layer:

  1. Case-insensitive lookups via LOWER() instead of COLLATE NOCASE.
    COLLATE NOCASE is SQLite-specific. Replace it with LOWER(col) = LOWER(?)
    predicates and matching functional LOWER(col) indexes in the models, plus a
    migration (0002_lower_indexes) that converts the legacy NOCASE indexes on
    existing SQLite databases. Also drop the MySQL-specific longblob column-type
    tags.

  2. Row locking in read-modify-write transactions.
    Add SELECT ... FOR UPDATE (clause.Locking{Strength: "UPDATE"}) to the
    transactions that read a row, mutate it in Go, and write it back, so concurrent
    updates of the same row are serialized and can't lose writes.

Why

This improves portability of the existing SQL layer and is a prerequisite for adding a
concurrent backend such as PostgreSQL [#753].

On SQLite both changes are behavior-preserving: LOWER() and NOCASE both fold
ASCII, and the single-writer connection pool already serializes writes, so the
FOR UPDATE lock is effectively a no-op there. The value shows up on any backend
that permits concurrent writers, where the lock prevents lost-update races.

The 0002_lower_indexes migration drops and recreates unique indexes on existing SQLite databases; this is safe because NOCASE and LOWER() both fold ASCII, so any row unique under the old index stays unique under the new one.

Testing

  • go test -tags testing ./database/sql/... ./config/... passes.
  • sqlmock query expectations updated to the new LOWER() SQL.
  • Verified on a fresh SQLite DB that the functional indexes are created and the
    planner uses them (e.g. SEARCH organizations USING INDEX idx_org_name_nocase).

Out of scope

The file-object/blob storage layer (file_store.go, the FileBlob /
FileObjectTag models, and their migration) is deliberately not modified in
this PR because the layer is tied to SQLite specifics; its conversion is
handled in a follow-up PostgreSQL PR.

Replace SQLite-only COLLATE NOCASE with functional LOWER() predicates and
indexes for case-insensitive lookups, and add a migration converting the
legacy NOCASE indexes on existing SQLite databases. Drop MySQL-specific
longblob column type tags.

Add SELECT ... FOR UPDATE row locking (clause.Locking{Strength: "UPDATE"})
to read-modify-write transactions so concurrent updates of the same row are
serialized. On SQLite the single-connection pool already serializes writes,
so the lock is a no-op; the change is groundwork that fixes lost-update races
on any concurrent RDBMS backend.

sqlmock query expectations updated to match the new SQL.
@gabriel-samfira gabriel-samfira merged commit 555215c into cloudbase:main Jun 1, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants