Skip to content

fix(datagrid): make the grid, the date picker and the write-back agree on a value - #2471

Merged
datlechin merged 1 commit into
mainfrom
fix/temporal-editing-consistency
Aug 26, 2026
Merged

fix(datagrid): make the grid, the date picker and the write-back agree on a value#2471
datlechin merged 1 commit into
mainfrom
fix/temporal-editing-consistency

Conversation

@datlechin

Copy link
Copy Markdown
Member

Found while investigating #2454. Unlike the Snowflake work that issue produced, these are app-side and affect every driver, so this is based on main rather than stacked.

Three defects with one shape: the grid, the cell editor and the write-back each decided for themselves what a temporal value meant, and they disagreed.

A time you typed was thrown away

The picker chooses its fields from the column type, but DateEditingService.string(from:like:) chose what to write from the shape of the stored text:

if layout.hasDate && layout.hasTime { ... } else if layout.hasDate { result = datePart }

A SQLite column declared DATETIME holding 2024-03-15 showed hour, minute and second steppers, because DATETIME maps to .dateAndTime. Setting 14:30:00 and pressing OK wrote 2024-03-15, identical to the stored value, so the commit was discarded as a no-op. No error, no dirty marker, no redraw. The time simply vanished.

The write-back now takes the fields the editor actually offered as well as the shape the value arrived in, so a field the user was allowed to fill is a field that gets written.

The grid and the picker showed different days

DateFormattingService set its formatter's zone once, at creation:

formatter.timeZone = TimeZone.current

while the picker opened the value in the zone the value itself carried. A PostgreSQL timestamptz rendered by the server as 2024-03-15 01:00:00+05:30, viewed on a Mac in Europe/London, read 2024-03-14 19:30:00 in the cell and opened on 15 March in the calendar. The same row, two days, and no way to tell which one the database held.

The value is now formatted in the zone it was written in. That is what DatabaseDateParser already documents as the intent: the zone travels with the value so the grid, the chart's axis and the picker all show it as written. A value with no offset is naive and still resolves in the reader's own zone, so nothing changes for DATETIME and friends.

The cache key needs no zone, because the zone is read off the value and the string therefore determines it.

Opening a picker and pressing OK rewrote the cell

onCommit always synthesized a string and committed it, so merely opening the editor and confirming could replace the stored value. The clearest case is a wall clock inside the reader's spring-forward gap: 2024-03-10 02:30:00 in a MySQL DATETIME is legal and common when the data was written elsewhere, but Calendar rolls it to 03:30, and confirming unchanged committed 2024-03-10 03:30:00, moving the row an hour.

Confirming without changing the value now writes nothing, which is the rule #2454 asked for.

Tests

TemporalEditingConsistencyTests, 7 cases: an entered time survives on a date-only value, the arrival shape still drives the default when no field set is offered, a date-only editor does not truncate a value carrying time, an offset suffix survives the round trip, an offset-bearing value formats on its own day, a naive value still reads in the reader's zone, and formatting one value does not change how the next reads (the shared formatter's zone does not leak).

Verified: build PASS, 108/108 across every suite that touches DateFormattingService, CellDisplayFormatter, ResultChartProjector, DateEditingService and DatabaseDateParser, SwiftLint 0 violations.

No UI automation. The flows need a cell whose stored text disagrees with its column type and a Mac in a specific time zone, neither of which the UI test harness can set deterministically. The behaviour is pinned at the three services instead.

Two related things deliberately left alone

The DST-gap display. DatabaseDateParser.keepsItsDay compares only year, month and day, so a wall clock rolled forward by an hour still passes and the grid shows 03:30 for a stored 02:30. The comment above it defends that on purpose: such a value is still the day it says it is. Rejecting it would make those cells fall back to raw text. The damaging half, the silent rewrite, is fixed above; changing what the grid displays is a product decision.

The 1582 calendar. DatabaseDateParser builds a Foundation .gregorian calendar, which applies the Julian cutover, so a date from 1582-10-05 to 1582-10-14 does not parse and an older one resolves to a different absolute day. Snowflake, and ISO 8601, use the proleptic calendar. Making the shared parser proleptic changes behaviour for every driver at once and deserves its own change.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit ad23817 into main Aug 26, 2026
8 checks passed
@datlechin
datlechin deleted the fix/temporal-editing-consistency branch August 26, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant