m/paliad
1
0
Fork 0
Code Issues 151 Pull Requests 4 Actions 2 Packages Projects Releases Wiki Activity

Backup export: make ORDER BY drift-resistant (auto-discover stable columns, drop hardcoded names) #140

New Issue
Open
opened 2026-05-26 16:01:32 +00:00 by mAi · 0 comments
mAi commented 2026-05-26 16:01:32 +00:00
Collaborator
Copy Link

Why

On 2026-05-26 the backup generator failed with:

backup generate: export sheet "appointment_caldav_targets":
query: pq: column "calendar_binding_id" does not exist at column 74 (42703)

Root cause: internal/services/export_service.go:1566 hardcoded ORDER BY appointment_id, calendar_binding_id against a table whose real join column is binding_id (mig 101). One-character bug.

The immediate fix landed on main (commit on branch mai/paliadin/fix-backup-binding-column). But the broader issue: every sheet in orgSheetQueries() hardcodes the ORDER BY columns. Same risk lurks on every other table — the next time someone renames a column, the backup silently breaks until someone tries to run it.

m flagged it: "This needs to work and be flexible, too!"

Scope — make the exporter drift-resistant

Three levels, pick one:

  1. Strict-best-effort: For every sheet, ORDER BY id if the table has an id column (auto-detect via information_schema.columns); otherwise fall back to no ORDER BY (deterministic-but-pg-order) with a WARN log. Keeps the deterministic-output contract for tables with id; gives up on the others. Smallest change, no manual maintenance.
  2. Self-describing sheets: each sheet declares its desired ORDER BY columns; the exporter probes the live schema, drops any that don't exist, ORDER BYs whatever's left. Falls back to id (or nothing) when all named columns are gone.
  3. Schema-aware: scan information_schema.columns for the table; ORDER BY all not-null FK / pk columns in declared order. Most clever, most surface to test, most likely to surprise.

Recommended: (1) for the bulk, with explicit OrderBy []string per sheet where the natural order matters (e.g. caldav_sync_log keyed by occurred_at, id). Smallest blast radius.

Worker scope

  • Refactor orgSheetQueries() to return (sheetName, table, orderBy?) triples instead of free SQL strings. Build the SQL at runtime via a query-builder helper that:
    • SELECTs * FROM the named table
    • Appends ORDER BY only for columns that exist (check via a single cached information_schema.columns query at the start of each backup run).
    • Logs WARN on any drop so the next schema rename is visible.
  • Migrate all existing entries: most are ORDER BY id; the few with multi-column orders (caldav_sync_log, project_teams, my_pinned_projects, ref__countries, ref__holidays, appointment_caldav_targets, backups) declare their lists.
  • Tests: a unit test that runs against a fixture DB with one missing column; the exporter drops the dead column from the ORDER BY without crashing, logs the WARN.

Acceptance

  1. /admin/backups → "Generate" succeeds against the live DB. Every sheet renders rows.
  2. Add a DROP COLUMN x migration in a test DB → next backup still succeeds, log shows a WARN about the dropped column.
  3. Reverse: add a new column → next backup still succeeds (was never in the ORDER BY).
  4. go test ./internal/services/... green incl. the new fixture test.
  5. No backend behaviour change for the happy path (sort order identical for tables with intact columns).

Anti-patterns

  • DON'T rewrite the whole exporter — single helper that builds the SQL string. Keep the existing column-dropping (DropColumns), secret-redaction, sheet-grouping logic untouched.
  • DON'T touch deadline_rules writes or any other audit-triggered table; this is a read-only exporter refactor.
## Why On 2026-05-26 the backup generator failed with: ``` backup generate: export sheet "appointment_caldav_targets": query: pq: column "calendar_binding_id" does not exist at column 74 (42703) ``` Root cause: `internal/services/export_service.go:1566` hardcoded `ORDER BY appointment_id, calendar_binding_id` against a table whose real join column is `binding_id` (mig 101). One-character bug. The immediate fix landed on main (commit on branch `mai/paliadin/fix-backup-binding-column`). But the broader issue: **every sheet in `orgSheetQueries()` hardcodes the ORDER BY columns**. Same risk lurks on every other table — the next time someone renames a column, the backup silently breaks until someone tries to run it. m flagged it: "This needs to work and be flexible, too!" ## Scope — make the exporter drift-resistant Three levels, pick one: 1. **Strict-best-effort**: For every sheet, ORDER BY `id` if the table has an `id` column (auto-detect via `information_schema.columns`); otherwise fall back to no ORDER BY (deterministic-but-pg-order) with a WARN log. Keeps the deterministic-output contract for tables with `id`; gives up on the others. Smallest change, no manual maintenance. 2. **Self-describing sheets**: each sheet declares its desired ORDER BY columns; the exporter probes the live schema, drops any that don't exist, ORDER BYs whatever's left. Falls back to `id` (or nothing) when all named columns are gone. 3. **Schema-aware**: scan `information_schema.columns` for the table; ORDER BY all not-null FK / pk columns in declared order. Most clever, most surface to test, most likely to surprise. Recommended: **(1)** for the bulk, with explicit `OrderBy []string` per sheet where the natural order matters (e.g. `caldav_sync_log` keyed by `occurred_at, id`). Smallest blast radius. ## Worker scope - Refactor `orgSheetQueries()` to return `(sheetName, table, orderBy?)` triples instead of free SQL strings. Build the SQL at runtime via a query-builder helper that: - SELECTs * FROM the named table - Appends ORDER BY only for columns that exist (check via a single cached `information_schema.columns` query at the start of each backup run). - Logs WARN on any drop so the next schema rename is visible. - Migrate all existing entries: most are `ORDER BY id`; the few with multi-column orders (caldav_sync_log, project_teams, my_pinned_projects, ref__countries, ref__holidays, appointment_caldav_targets, backups) declare their lists. - Tests: a unit test that runs against a fixture DB with one missing column; the exporter drops the dead column from the ORDER BY without crashing, logs the WARN. ## Acceptance 1. `/admin/backups` → "Generate" succeeds against the live DB. Every sheet renders rows. 2. Add a `DROP COLUMN x` migration in a test DB → next backup still succeeds, log shows a WARN about the dropped column. 3. Reverse: add a new column → next backup still succeeds (was never in the ORDER BY). 4. `go test ./internal/services/...` green incl. the new fixture test. 5. No backend behaviour change for the happy path (sort order identical for tables with intact columns). ## Anti-patterns - DON'T rewrite the whole exporter — single helper that builds the SQL string. Keep the existing column-dropping (`DropColumns`), secret-redaction, sheet-grouping logic untouched. - DON'T touch `deadline_rules` writes or any other audit-triggered table; this is a read-only exporter refactor.
mAi self-assigned this 2026-05-26 16:01:32 +00:00
mAi added the
done
 label 2026-05-26 16:20:54 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
deferred
Parked — not pursuing now, may un-park later
done
Work completed by an agent; awaiting m to close
No Label
done
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
mAi m
No Assignees mAi
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: m/paliad#140
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?