Repository navigation
Description
Follow-up to #123 (raised by CodeRabbit's review of that PR, and disclosed by the author in its description).
Problem
expr({:datetime_add, ...}) renders the cutoff with strftime('%Y-%m-%dT%H:%M:%SZ', ...), which truncates to whole seconds. Because these comparisons are textual, a :utc_datetime_usec or :naive_datetime_usec value landing inside the cutoff second sorts on the wrong side of it: "...01.900000Z" compares less than "...01Z", since . (0x2E) precedes Z (0x5A).
Reproduced against a real database:
stored: ~U[2026-09-18 03:46:01.900000Z]
cutoff param: ~U[2026-09-19 03:46:01.709191Z] → minus 1 day = ...01.709191
rendered SQL: (s0."at_usec" > strftime('%Y-%m-%dT%H:%M:%SZ', ?1, CAST(-1 AS TEXT) || ' days'))
result: row dropped, although .900000 > .709191
Both :utc_datetime_usec and :naive_datetime_usec are affected. The error is bounded at one second and only bites when a row ties the cutoff second, which is why #123 was still a large net improvement — before it, the interval was discarded entirely and every ago/2 degraded to > now().
Why the obvious fix does not work
Switching the format to %f (milliseconds) fixes the usec types but regresses :utc_datetime. Truth here is that the .900000 rows should match and the whole-second rows should not:
| stored shape | truth | current %S |
with %f |
|---|---|---|---|
utc_datetime_usec ...01.900000Z |
match | ✗ drops | ✓ fixed |
utc_datetime ...01Z |
no match | ✓ correct | ✗ wrongly matches |
naive_usec ...01.900000 |
match | ✗ drops | ✓ fixed |
naive ...01 |
no match | ✓ correct | ✓ correct |
Once the cutoff carries a fraction, "...01Z" > "...01.709Z" holds textually, so rows genuinely before the cutoff start matching — a silent wrong-direction error in a more widely used type than the one being repaired.
No single text format satisfies both: usec-with-Z needs the cutoff to carry a fraction, second-precision-with-Z needs it not to.
Proposed approach
Normalise both operands numerically instead of comparing text. Verified correct for all four stored shapes (SQLite 3.51):
julianday(val) > julianday(cutoff) -- 1, 0, 1, 0 (matches truth)
unixepoch(val,'subsec') > unixepoch(cutoff,'subsec') -- same, but needs SQLite >= 3.42julianday/1 is the safer of the two on version grounds. Its resolution is roughly 8-9µs (a double spanning ~2.46M days), so it is not exact to the microsecond but is ~10^5 times tighter than the current one-second error.
The catch
This cannot be done inside the datetime_add clause. That clause only renders its own subtree — the right-hand side — while the comparison operator renders each operand independently. Making this work needs one of:
- Datetime-aware comparison operators —
>,<,>=,<=detect a datetime operand and wrap both sides. Most general, largest blast radius, and needs care not to disturb non-datetime comparisons. - Type-directed rendering — the renderer learns the column's Ecto type and emits a cutoff whose shape matches the stored format exactly. Narrower, but requires type information that
expr/3does not currently carry.
Option 1 also fixes ordinary datetime column comparisons that are not ago/2-related, if they have the same text-shape mismatch; worth checking whether they do.
Acceptance criteria
ago/2andfrom_now/2compare correctly for:utc_datetime,:utc_datetime_usec,:naive_datetimeand:naive_datetime_usec.- Boundary tests covering a row inside the cutoff second on both sides of it, for each of those four types.
- No regression in
test/ecto_datetime_interval_test.exs(added by Support datetime_add and date_add so ago/2 keeps its interval #123).
Activity
ocean commentedon Sep 19, 2026
CodeRabbit's CLI raised this again when reviewing the merged
lib/changes, and proposed an option not covered above:That is worth adding as a third approach alongside the two already listed:
CLAUDE.md.It has a real advantage over the
julianday/1route. Every bug in this cluster — the dropped interval, the unbound arithmetic placeholder, this precision loss — has the same shape: the adapter silently returns a wrong answer instead of failing.julianday/1narrows the error from one second to roughly 8-9µs but keeps it silent and approximate; rejecting outright converts it into something the caller can see. Given the repository already has a documented convention for unsupported functionality, that fits the codebase better than a lossy numeric comparison.The obvious cost is that it breaks working code for anyone comparing a
:utc_datetime_useccolumn today, where the result is right except within the cutoff second. So it is probably only acceptable combined with option 1 or 2 — normalise where it can be done correctly, reject only the variants that genuinely cannot be.Worth deciding between these before any implementation starts, since they lead to quite different diffs.
ocean commentedon Sep 19, 2026
Correction to the proposed approach above. The claim that
julianday/1compares all four shapes correctly, with "roughly 8-9µs" resolution, is wrong. That figure was inferred from double precision rather than measured, and measuring it changes the conclusion.SQLite's date and time functions work to millisecond precision and round at parse time:
The microseconds are discarded before any comparison happens:
juliandayseparates?unixepoch(…,'subsec')My earlier four-shape table was run with a one-second delta, which every approach gets right, so it did not exercise the thing that actually matters.
This rules out option 1 entirely — the precision is lost inside SQLite regardless of which date function wraps the operands, so there is no numeric normalisation to reach for. Option 2 is ruled out for the same reason:
strftimecannot emit a microsecond cutoff,%fstopping at milliseconds.It also makes the other half clearer: second-precision columns are already exact.
:utc_datetime,:naive_datetimeand:datestore a shape that compares correctly as text against the rendered cutoff, boundary included, so there was never anything to normalise for them.So "normalise where it can be done correctly, reject where it cannot" collapses to just the rejection, because the normalising half is already true. #131 implements that:
:utc_datetime_usecand:naive_datetime_usecraise when compared againstago/2/from_now/2, everything else is untouched, and the guard is scoped to comparisons involving adatetime_add/date_addnode so ordinary comparisons against bound values keep working on those columns.