fix: write and compare sqlite index timestamps in UTC (#1497) - #1498
Merged
Conversation
…#1497) SqliteIndexDAO writes start_time/update_time via Timestamp.toString(), which renders in the JVM default zone, while SqliteIndexQueryBuilder renders search bounds in UTC. On a negative-offset host this makes recent rows invisible to time-range searches. - Pin the sqlite-persistence test task to TZ=America/Asuncion so the regression is visible; it's a no-op on UTC CI otherwise. - SqliteIndexDAOTest: add searchesFindWorkflowsIndexedInANonUtcZone and searchesFindTasksIndexedInANonUtcZone (currently 0 hits instead of 1), and switch the stored-text assertions in compareWorkflowSummary/ compareTaskSummary from Timestamp.toString() to the canonical UTC format the fix will use. - SqliteIndexQueryBuilderTest: add a test asserting the datetime() wrapper is dropped and the bound parameter is rendered in canonical UTC text. - Add SqliteIndexTimestampMigrationTest, which loads the not-yet-created V6__index_timestamps_to_utc.sql from the classpath and fails with an explicit "migration file not found" message until that migration ships. All 15 new/changed assertions are red for the right reason (timezone/format mismatch or missing migration file), not compile errors.
SqliteIndexDAO wrote start_time/update_time via java.sql.Timestamp.toString(),
which renders in the JVM default zone, while SqliteIndexQueryBuilder rendered
search bounds in UTC. Because the columns are TEXT-compared, every time-range
search was off by the host's UTC offset (on America/Asuncion, -4h).
Write and read paths now share a canonical yyyy-MM-dd HH:mm:ss.SSS UTC format
that is byte-identical to SQLite's strftime('%Y-%m-%d %H:%M:%f', ...). Also
drop the datetime() wrapper in the read path, since it truncated to whole
seconds and could wrongly exclude rows with a fractional-second lower bound.
A new V6 migration rewrites existing rows to the canonical UTC text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The zone only needs to be non-UTC with DST for these regression tests to be meaningful. Asuncion was the reporting host's zone, which is needless local color in a shared build file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The byte-for-byte rationale is already stated on toSqliteUtcTimestamp and in the V6 migration; repeating it over each field constant added noise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mp-orkes
force-pushed
the
fix/sqlite-index-utc-timestamps
branch
from
August 7, 2026 03:07
63f3720 to
9c36c68
Compare
…ne (#1497) The V6 migration used SQLite's 'utc' modifier to reinterpret the stored local-time text. That reads the host tz database, but the bad values were written against the JVM's bundled tz database, and the two disagree whenever the JVM's tzdata is older than the OS's. Observed on JDK 21 (tzdata 2023c) against macOS (tzdata 2026c): Paraguay dropped DST in tzdata 2024b, so the JVM wrote America/Asuncion at -04:00 while SQLite read it back at -03:00. Every migrated row landed an hour early, which is enough to hide recent executions from a "last 30 minutes" search. start_time/update_time are derived data: json_data already carries the authoritative instant as an ISO-8601 string that SQLite parses natively. Rebuild the columns from it and no timezone is consulted at any point. This is exact under tzdata skew, exact when a DB moves between hosts in different zones (a limitation the previous version documented as unfixable), and self-correcting for rows an earlier attempt already shifted wrongly. Adds a json_valid() guard: json_extract() raises "malformed JSON" and aborts the whole statement, which would have failed the migration and stopped the server from booting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
This is happening with SQLite. sqlite-tz-issue.mp4 |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mp-orkes
requested review from
NicholasDCole,
bradyyie,
kowser-orkes,
manan164 and
v1r3n
August 7, 2026 03:45
mp-orkes
marked this pull request as ready for review
August 7, 2026 03:47
v1r3n
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1497 - take a look at the screen recording for a repro of the issue.
What changed
SqliteIndexDAOwrotestart_time/update_timeviajava.sql.Timestamp.toString(), which renders in the JVM default zone, whileSqliteIndexQueryBuilderrendered the search bound in UTC. These columns are TEXT-compared, so every time-range search was wrong by the host's UTC offset.Both sides now use one canonical format,
yyyy-MM-dd HH:mm:ss.SSSin UTC, applied to all four index timestamp columns (workflow_indexandtask_index×start_time/update_time).V6__index_timestamps_to_utc.sqlconverts existing rows.How to test
The suite runs under
TZ=America/Los_Angeles(set insqlite-persistence/build.gradle). This is deliberate and load-bearing — CI runs UTC, where a local-time/UTC mismatch is invisible and every one of these tests passes vacuously.To see it red, check out the first commit alone (tests without the fix): 15 failures, including the reported symptom as
expected:<1> but was:<0>..🤖 Generated with Claude Code