fix(datagrid): read every database date spelling with one grammar - #2249
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
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
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 #2241.
TablePro had two grammars for the same job, recovering a value from the date text a driver puts on the wire.
DatabaseDateParserheld nine orderedDateFormatterpatterns for the grid and the chart's time axis;DateEditingServiceheld a regex for the cell editor. Nothing forced them to agree, and they had already drifted.This gives the grammar to one type.
DatabaseDateParsernow owns the regex,TemporalLayoutandParsedTemporalValue, and answers both questions:date(from:)for display and the chart,parse(_:)for the editor.DateEditingServicekeeps only the write side.Two live bugs the split was hiding
An empty date cell opened the picker at UTC. With no text to read a zone from, the picker fell back to
.gmtanddefaultStringwrote in.gmt, so the value written was the user's clock shifted by their offset. Measured at UTC+7: picking "now" at 06:00 local opened the picker at 23:00 and wrote2024-03-14into a DATE column, the previous day. The offset is wrong at every hour; the day is wrong for the hours after local midnight. Both now resolve in the user's own zone.A time carrying an offset rendered as raw text.
09:30:00+07, which PostgreSQLtime with time zoneproduces, matched the editor's regex but none of the nine formatter patterns. So the cell was editable as a date but never formatted as one. One grammar closes that by construction.The naive-value zone is settled deliberately, per the issue. Both readers now use
TimeZone.current: display already formats in it (DateFormattingService), the chart axis renders in it, and the picker is handed the zone explicitly, so a naive value shows and writes as written.Two regressions caught while building this, worth reading
I would have shipped both without measuring, so they are recorded here rather than buried.
DateFormatteraccepts an unpadded2024-9-1; my first regex required two digits and silently stopped such a value being a date at all. Month, day and hour now take one or two digits.Calendarrolls an out-of-range component over instead of refusing it, whereDateFormatterrefused. MySQL's0000-00-00 00:00:00became a plausible0002-11-30, and2024-13-45became2025-02-14. Both used to render as text and do again:isInRangerejects the impossible ones andkeepsItsDaycatches2024-02-30, which onlyCalendarcan detect. The day check deliberately ignores the clock, so a naive time inside a spring-forward gap is still the day it says it is.Fractional seconds
They now reach the
Date, at full precision rather than the millisecond truncationSSSSSSgave.ResultChartProjectoruses theDateas its grouping key, so two rows at.100and.200were sharing one x position. The text is kept verbatim in the layout and re-emitted on write, soDATETIME(6)still round-trips character for character.oneGrammarServesDisplayAndEditingasserts that for all 15 spellings.Verification
DatabaseDateParserTests,DateEditingServiceTests,ResultChartProjectorTests: 52 executed, 52 passed. Log grepped forFailing tests:,TEST FAILEDandCrash:, 0 hits.swiftlint --strictclean on all 8 changed files.swiftcprobes drove the design: one compared both grammars across 15 wire spellings and found thetimetzdrift, the other measured the empty-cell zone bug at UTC+7. Neither is committed; they answered a question rather than pinning a constant.A test that crashed instead of failing
While the padding regression was live,
ResultChartProjectorTests.dateAxisIsTemporalsubscripteddates[2]after an#expect(dates.count == 3).#expectdoes not stop execution, so the array read went out of bounds and killed the test process, marking 12 unrelated tests as failed with no message and a different set each run. That cost two cycles to diagnose. It is nowtry #require, so a failure stays one test.Noted, not fixed
An offset-bearing value disagrees with its own cell:
2024-03-15T09:30:00Zdraws as16:30in the grid (converted to your zone) while the picker opens at09:30(as written). Both are defensible and changing either is a product call, so this PR leaves it alone.