From 2484cd2515e2e86168cc43e8b6d31df42e537b75 Mon Sep 17 00:00:00 2001 From: Gregory Salaun Date: Thu, 13 Aug 2026 00:37:53 +0200 Subject: [PATCH] fix(log): the QSO number column was always empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OrderedIDs scanned qso_date straight into a time.Time. The column holds a formatted STRING — the repo writes it with Format(isoMillis) and reads it back through parseTimeLoose everywhere else — so the scan failed on every call, the index was never built, and omitempty then dropped the zero from the JSON. A column that shipped blank. Scans the string and parses only the newest row; the ordering already came from SQL, so 30 000 parses were never needed. Tested against a real SQLite file with the date stored exactly as the repo writes it, and with the ids running opposite to the dates — the imported-ADIF case that is the whole reason this is not the id. It fails without the fix with the scan error itself. --- internal/qso/orderedids_test.go | 64 +++++++++++++++++++++++++++++++++ internal/qso/qso.go | 15 +++++--- 2 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 internal/qso/orderedids_test.go diff --git a/internal/qso/orderedids_test.go b/internal/qso/orderedids_test.go new file mode 100644 index 0000000..e9bfc94 --- /dev/null +++ b/internal/qso/orderedids_test.go @@ -0,0 +1,64 @@ +package qso + +import ( + "context" + "database/sql" + "path/filepath" + "testing" + "time" + + _ "modernc.org/sqlite" +) + +// OrderedIDs against a real SQLite file, with qso_date stored the way the repo +// actually writes it. +// +// The first version scanned that column straight into a time.Time. It compiles, +// it reads correctly, and it fails at run time — the column holds a formatted +// STRING. The error was swallowed into "the column will be empty", which is +// exactly what shipped: a column that was always blank. +func TestOrderedIDsAgainstRealSQLite(t *testing.T) { + conn, err := sql.Open("sqlite", "file:"+filepath.Join(t.TempDir(), "t.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { conn.Close() }) + if _, err := conn.Exec(`CREATE TABLE qso ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + callsign TEXT NOT NULL, + qso_date TEXT NOT NULL + )`); err != nil { + t.Fatal(err) + } + + // Inserted NEWEST first, so the ids run opposite to the dates — what an + // imported ADIF produces, and the whole reason the id is not the number. + for _, row := range []struct { + call string + at time.Time + }{ + {"NEWEST", time.Date(2026, 8, 13, 10, 0, 0, 0, time.UTC)}, + {"MIDDLE", time.Date(2020, 1, 1, 10, 0, 0, 0, time.UTC)}, + {"OLDEST", time.Date(1999, 5, 5, 10, 0, 0, 0, time.UTC)}, + } { + if _, err := conn.Exec(`INSERT INTO qso (callsign, qso_date) VALUES (?, ?)`, + row.call, row.at.UTC().Format(isoMillis)); err != nil { + t.Fatal(err) + } + } + + ids, newest, err := NewRepo(conn).OrderedIDs(context.Background()) + if err != nil { + t.Fatalf("OrderedIDs: %v — this is the failure that emptied the column", err) + } + if len(ids) != 3 { + t.Fatalf("got %d ids, want 3", len(ids)) + } + // Rows 1,2,3 were inserted newest→oldest, so chronological order is 3,2,1. + if ids[0] != 3 || ids[1] != 2 || ids[2] != 1 { + t.Errorf("order = %v, want the oldest contact first (3,2,1)", ids) + } + if newest.Year() != 2026 { + t.Errorf("newest = %v, want the 2026 contact", newest) + } +} diff --git a/internal/qso/qso.go b/internal/qso/qso.go index 3bfbe58..fe3dfea 100644 --- a/internal/qso/qso.go +++ b/internal/qso/qso.go @@ -3195,15 +3195,22 @@ func (r *Repo) OrderedIDs(ctx context.Context) ([]int64, time.Time, error) { } defer rows.Close() out := make([]int64, 0, 4096) - var newest time.Time + // qso_date is stored as a formatted STRING, not a driver date. Scanning it + // into a time.Time silently fails on SQLite, which is how this returned an + // error and left the whole column empty. + var lastDate string for rows.Next() { var id int64 - var at time.Time + var at string if err := rows.Scan(&id, &at); err != nil { return nil, time.Time{}, err } out = append(out, id) - newest = at // ascending, so the last row read is the newest + lastDate = at // ascending, so the last row read is the newest } - return out, newest, rows.Err() + if err := rows.Err(); err != nil { + return nil, time.Time{}, err + } + // Parsed once, for the newest row only — the ordering came from SQL. + return out, parseTimeLoose(lastDate), nil }