Skip to content

Proposal: optional PostgreSQL support, alongside SQLite #28

Description

@distronode-com

We are running Calnode on Cloud Run and would like to move the store to PostgreSQL. We have a
working branch and would rather agree the shape with you before opening a large pull request.

Nothing below changes SQLite behaviour. Single-binary-plus-a-file stays the default and the
documented path; Postgres is opt-in via DATABASE_URL.

Why we want it

Cloud Run scales to zero and gives an instance an ephemeral filesystem, so SQLite lives on an
in-memory volume with Litestream replicating to object storage. That works, and we have verified
a real restore after an idle scale-down. But it forces --max-instances 1 permanently: two
instances would replicate two copies of the same file into one bucket. Postgres is what lets a
deployment scale horizontally without changing anything else.

If you would rather Calnode stay single-engine, say so and we will keep this in our fork. That is
a legitimate answer and we would not be offended.

What the work actually is

sqlc.yaml points at db/queries, which does not exist, and internal/db/sqlcdb is empty, so
every statement is hand-written: 763 Query/QueryRow/Exec call sites across 124 files,
453 of them outside tests in 74 files. The approach that made this tractable:

  • One choke point. A *db.DB / *db.Tx wrapper whose method set matches database/sql
    exactly and rebinds ? to $n when the dialect is Postgres. Because the names match, the
    763 call sites needed no edit at all
    — only the declarations that hold the handle changed.
  • Rebinding is a small lexer, not a string replace, so a ? inside a string literal, a
    quoted identifier or a comment is left alone. Nested block comments are honoured because
    Postgres nests them.
  • Per-dialect goose migration directories, the Postgres set generated once from the 57
    existing files.

The three decisions we would most like your opinion on

1. Booleans stay integers. The schema stores flags as INTEGER NOT NULL DEFAULT 0/1 and the
tree compares them literally (email_login = 1, boolToInt(...)). We translated those to
SMALLINT, not BOOLEAN, so no scan target changes. BOOLEAN would be tidier and would touch
a great many call sites.

2. Timestamps stay TEXT. Every time value is a string end to end — WHERE run_at <= ?,
ORDER BY created_at, (start_at, id) pagination keys — so we translated
datetime('now') to to_char(now() AT TIME ZONE 'UTC', ...) rather than adopting TIMESTAMPTZ.
With TIMESTAMPTZ, database/sql hands a time.Time to a *string as RFC3339Nano, which sorts
differently from the stored SQLite form and would silently change API output.
⚠️ One caveat we have not resolved: TEXT timestamps are compared lexicographically, which is byte
order on SQLite but depends on the server's collation on Postgres. Mixed shapes (space-separated
vs T-separated) may not order identically. COLLATE "C" would settle it.

3. The single connection is a correctness guarantee, and it needed replacing.
docs/ARCHITECTURE.md is explicit that the app-level booking-overlap check is free of TOCTOU
races only because SetMaxOpenConns(1) serialises every transaction. That property does not
survive a pool. We take pg_advisory_xact_lock on the host id inside the booking and reschedule
transactions, released automatically at commit.

We proved it with a negative control rather than asserting it. Two goroutines racing overlapping
bookings for one host, 40 rounds:

created conflicts overlapping pairs
advisory lock in place 40 40 0
lock disabled 79 1 39

The unlocked run is the interesting half: idx_bookings_no_double caught 1 race in 40, because a
partial unique index on (host_id, start_at) only catches an exact start-time collision, not a
partial overlap. We also cover ReassignHost, which has the same check-then-write shape.

One thing our change breaks that we would fix in the same PR

Splitting the migrations into per-dialect directories makes .github/workflows/audit.yml:98
match nothing — it globs internal/db/migrations/*.sql. Because grep -r on the unexpanded
literal exits 2, the if is simply false and the job still passes, so the tenant/workspace
column guard would become silently vacuous. audit/claims.yaml's single-binary-no-server-db
claim also becomes false, and its own check greps go.mod for postgres|mysql|redis, none of
which match github.com/jackc/pgx/v5. Both want a deliberate decision from you rather than a
quiet edit from us.

A smaller bug we found on the way, worth fixing regardless of this proposal

Constraint violations are classified by matching SQLite's English error text
(UNIQUE constraint failed and friends) in 13 places across 7 filesinternal/booking/service.go
and, in internal/handler, event_type.go, booking_handler.go, teams.go, override.go,
idempotency.go, availability.go. On any engine or locale where that text differs the
handler falls through to a 500 instead of a 409/400/404. It is also unnecessary on SQLite:
modernc.org/sqlite exposes Code(), and the extended result codes are populated —
SQLITE_CONSTRAINT_UNIQUE 2067, _PRIMARYKEY 1555, _CHECK 275, _FOREIGNKEY 787.
⚠️ Note 1555 and 2067 both produce the message "UNIQUE constraint failed", so a switch to codes
must accept both or it silently stops recognising primary-key collisions.

Happy to send that as its own small PR first if you would prefer to take it independently.

Where the branch stands

go test ./... is green on both engines from the same tree — SQLite (the default, nothing
set) and PostgreSQL 17 via CALNODE_TEST_POSTGRES_DSN. gofmt -l is empty and go vet ./... is
clean.

Two things worth saying about how that was checked, because the first version of it lied to us:

  • A green Postgres run is indistinguishable from a silently skipped one, since the helpers skip
    when the DSN is unset. So it is confirmed with a positive control (the ten
    TestPostgres_* cases are asserted to RUN, not skip) and a negative control (a deliberately
    wrong password must fail the package rather than skip it — it does, SQLSTATE 28P01).
  • Per-package green was not enough. Two of us ran the suite in pieces and both came up clean while
    13 call sites were still classifying constraint violations by SQLite's English error text;
    only running the whole suite against both engines from one tree surfaced them. That is why the
    error-code change above is in this proposal at all.

How we would like to proceed

Split into reviewable pieces rather than one enormous diff:

  1. the dialect layer and the rebinding wrapper, with SQLite behaviour unchanged;
  2. the generated Postgres migrations;
  3. the call-site conversion and the dialect-specific SQL;
  4. the advisory lock and its concurrency test;
  5. CI running the suite against a Postgres service container.

Tell us which of these you would take, in what order, and we will follow your preferences on the
three decisions above.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions