Artifact 39a8aea70f64e5ce0de12079a3bf4701938c03a4fb0b7b18cbcd1342ffe8dec1:
- File
db/scan_test.go
— part of check-in
[78a793dd83]
at
2026-07-23 11:17:26
on branch trunk
— fix: cover ScanJulianDay's ordering consumers, keep the float64 branch exact (#60) (#121)
Issue #60's triage was stale: the live truncation bug it describes was already closed on main by 11967ff (#62), which added millisecond rounding to timeToJulian. What remained was a coverage gap, not a defect.
An earlier attempt on this branch also normalized the float64 branch onto the millisecond grid, on the theory that a driver-independent helper is strictly better. That was wrong, and this commit does not do it. A scanned float64 IS the value stored in the row, so snapping it can only move it away from what the row holds. Timeline's pagination cursor depends on the opposite property: its predicate `e.mtime < ? OR (e.mtime = ? AND b.rid < ?)` requires the cursor to compare exactly equal to the row it came from. SQLite's own julianday() computes iJD/86400000.0, a different expression from TimeToJulian's julianEpoch + ms/86400000.0, and the two land one ulp apart on a real fraction of instants — so normalizing alters mtimes written by upstream Fossil, the library's primary real-world input. Raised, the cursor row repeats as the next page's first row; lowered, rows are skipped. Both are silent wrong answers.
The float64 branch therefore stays verbatim, and its doc comment now records why that exactness is load-bearing rather than incidental. The same reasoning applies to tag propagation's `tagxref.mtime < ?`, where SQLite compares the bound value against raw stored floats: an exact bound is the correct one there too, so no normalized variant is introduced for it either.
The one genuine transform stays where it belongs, in the time.Time branch, which must round because ncruces hands back DATETIME columns carrying its own sub-millisecond conversion noise. Its comment now also records the limit of that recovery: an mtime written at finer than millisecond resolution cannot be reproduced through a time.Time at all, and rounding the float64 branch to match would discard precision on both drivers rather than recover any.
Non-functional cleanup: millisPerDay and julianDayFromMillis replace two open-coded copies of the same expression.
Coverage added (no build tag; these run under both drivers): - db: TestScanJulianDayFloatBranchIsBitExact sweeps julianday()-computed mtimes and asserts each scans back bit-for-bit, with a guard that fails the test if the sweep ever stops containing divergent values. Verified to fail against the normalizing version and pass against this one. - db: TestScanJulianDayIsDriverIndependent proves the float64 and time.Time representations of one millisecond-aligned write converge, with noise in both directions. TestScanJulianDaySentinelZero pins the "no mtime" sentinel that bisect's `mtime != 0` guard depends on. - tag: TestPropagateAllOrderingAdjacentMillisecond drives the `tagxref.mtime < ?` decision through ScanJulianDay with adjacent-millisecond mtimes; it fails under a truncating conversion and passes under the rounding one, and passes under modernc regardless — the reason this class went unnoticed twice.
Consumer classification (ScanJulianDay): - manifest/timeline.go NewCursor: ordering/equality, requires exactness - tag/propagate.go PropagateAll -> `tagxref.mtime < ?`: ordering (now covered) - tag/propagate.go childMtime -> mtimeQueue.Less: ordering, traversal-order only (visited set makes it result-neutral) - manifest/log.go, bisect/bisect.go: display-only
Known limitation, left alone deliberately: julianToTime truncates where timeToJulian rounds, so the two are not inverses at sub-millisecond resolution. Pre-existing, now documented; ScanTime's consumers are display paths with no ordering boundary. This commit does not widen the gap. (user: [email protected], size: 5832) [annotate] [blame] [check-ins using]
package db import ( "testing" "time" ) // sqliteJulianDay reproduces the expression SQLite's own julianday() evaluates: // it stores an instant as integer milliseconds since julian day 0 (iJD) and // returns iJD/86400000.0. That is a different expression from the one // TimeToJulian writes with (julianEpoch + ms/86400000.0), and the two land one // ulp apart on a real fraction of instants — which is the whole reason the // float64 branch of ScanJulianDay must not "canonicalize" what it is handed. // // 210866760000000 is iJD for the Unix epoch (2440587.5 days * 86400000 ms). func sqliteJulianDay(unixMillis int64) float64 { return float64(210866760000000+unixMillis) / millisPerDay } // TestScanJulianDayFloatBranchIsBitExact is the cursor-safety regression. // // Timeline's pagination cursor carries the mtime float64 exactly as read off // the row, and its predicate `e.mtime < ? OR (e.mtime = ? AND b.rid < ?)` // depends on that value comparing exactly equal to the row it came from. If // ScanJulianDay perturbs a scanned float64 by even one ulp, the boundary breaks // silently and in both directions: raise it and the last row of a page repeats // as the first row of the next; lower it and rows are skipped entirely. // // The values that expose this are not exotic — they are every mtime written by // upstream Fossil via SQLite's julianday(), i.e. the library's primary // real-world input. This test sweeps such values, first asserting the case is // genuinely divergent (so it can never pass vacuously) and then asserting // ScanJulianDay returns each one verbatim. func TestScanJulianDayFloatBranchIsBitExact(t *testing.T) { base := time.Date(2024, 3, 9, 14, 25, 17, 123_000_000, time.UTC).UnixMilli() var divergent int for i := 0; i < 5000; i++ { ms := base + int64(i)*1000 stored := sqliteJulianDay(ms) if stored != julianDayFromMillis(ms) { // This instant is one where julianday() and TimeToJulian disagree // — exactly the input a millisecond-snapping normalization would // alter, and therefore the input that breaks the cursor. divergent++ } got, ok := ScanJulianDay(stored) if !ok { t.Fatalf("ScanJulianDay(float64) returned ok=false for %.20f", stored) } if got != stored { t.Fatalf("ScanJulianDay altered a julianday()-computed mtime: got %.20f, want %.20f (bit-exact). "+ "The Timeline cursor compares this value against the row it came from; any change repeats or skips rows.", got, stored) } } // Guard the guard: if julianday() and TimeToJulian ever agreed on every // instant, the sweep above would prove nothing. if divergent == 0 { t.Fatal("no divergent julianday() values in the sweep — the test would pass vacuously; pick a different range") } t.Logf("%d of 5000 swept julianday() mtimes differ from the millisecond grid (these are the cursor-breaking inputs)", divergent) } // TestScanJulianDayIsDriverIndependent proves that a value this codebase wrote // scans back to the identical julian day regardless of which representation the // compiled-in driver hands back. modernc returns the column as a float64; // ncruces returns it as a time.Time carrying that driver's own sub-millisecond // REAL-conversion noise (~13us observed) around the same instant. // // Millisecond alignment is the precondition, not an incidental choice of // fixture: TimeToJulian rounds every mtime this codebase writes to whole // milliseconds, and that is what lets the time.Time branch recover the exact // original despite the noise. The perturbation below is deliberately // sub-millisecond — that is the only interval this bug class lives in, and it // is why two prior truncation bugs survived suites that only compared // timestamps hours apart. // // Note the asymmetry this test does NOT claim: for an mtime written at finer // than millisecond resolution (upstream Fossil via julianday()), the time.Time // branch cannot reproduce the stored value, because event.mtime is declared // DATETIME and ncruces converts it through time.Time before this code sees it. // That is a pre-existing driver limitation, not something the float64 branch // can or should compensate for — see TestScanJulianDayFloatBranchIsBitExact. func TestScanJulianDayIsDriverIndependent(t *testing.T) { instant := time.Date(2024, 3, 9, 14, 25, 17, 123_000_000, time.UTC) canonical := julianDayFromMillis(instant.UnixMilli()) // modernc-style: the column arrives as the float64 that was written. fromFloat, ok := ScanJulianDay(canonical) if !ok { t.Fatal("ScanJulianDay(float64) returned ok=false") } // ncruces-style: the same instant arrives as a time.Time carrying the // driver's sub-millisecond conversion noise, in both directions. for _, noise := range []time.Duration{300 * time.Microsecond, -300 * time.Microsecond} { fromTime, ok := ScanJulianDay(instant.Add(noise)) if !ok { t.Fatalf("ScanJulianDay(time.Time) returned ok=false (noise %v)", noise) } if fromTime != fromFloat { t.Fatalf("driver-dependent scan with %v noise: time.Time branch = %.20f, float64 branch = %.20f", noise, fromTime, fromFloat) } if fromTime != canonical { t.Fatalf("time.Time branch did not recover the written millisecond (noise %v): got %.20f, want %.20f", noise, fromTime, canonical) } } } // TestScanJulianDaySentinelZero asserts the "no mtime" sentinel survives // unchanged, since bisect's `mtime != 0` guard depends on an exact zero passing // through both the int64 (COALESCE(mtime, 0)) and float64 representations. func TestScanJulianDaySentinelZero(t *testing.T) { for _, v := range []any{int64(0), float64(0)} { got, ok := ScanJulianDay(v) if !ok { t.Fatalf("ScanJulianDay(%T(0)) returned ok=false", v) } if got != 0 { t.Fatalf("ScanJulianDay(%T(0)) = %.20f, want exactly 0", v, got) } } }