Export large-year datetimes with the timezone designator - #3136
Export large-year datetimes with the timezone designator#3136hannahbast wants to merge 2 commits into
Conversation
A date with a year outside [-9999, 9999] was exported without the timezone designator (for example, "-11700-01-01T00:00:00" for a value that was inserted as "-11700-01-01T00:00:00Z"), while dates within that range keep their timezone. For such large years, only the year is stored, so the original timezone is not available; export the canonical UTC form, consistent with the export of dates within the range.
There was a problem hiding this comment.
Pull request overview
This PR fixes RDF export of xsd:dateTime literals with years outside [-9999, 9999] so they retain a timezone designator (exported as canonical UTC Z) instead of being emitted without a timezone, which changes the literal’s meaning.
Changes:
- Export large-year
xsd:dateTimevalues with a trailingZinDateYearOrDuration::toStringAndType(). - Update large-year datetime parsing/round-trip tests to account for canonical UTC export (including when the input had no timezone).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/util/DateYearDuration.cpp |
Changes large-year xsd:dateTime serialization to include Z (UTC designator). |
test/DateYearDurationTest.cpp |
Updates tests to expect canonical UTC (Z) export for large-year datetimes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| testLargeYearDatetime("2039481726-01-01T00:00:00Z", 2039481726); | ||
| testLargeYearDatetime("-2039481726-01-01T00:00:00Z", -2039481726); | ||
| testLargeYearDatetime("2039481726-01-01T00:00:00", 2039481726, | ||
| "2039481726-01-01T00:00:00Z"); | ||
| testLargeYearDatetime("-2039481726-01-01T00:00:00", -2039481726, | ||
| "-2039481726-01-01T00:00:00Z"); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3136 +/- ##
==========================================
+ Coverage 92.44% 92.45% +0.01%
==========================================
Files 547 547
Lines 46547 46563 +16
Branches 6314 6314
==========================================
+ Hits 43032 43052 +20
Misses 1680 1680
+ Partials 1835 1831 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
I would agree with the change. Do I understand it correctly that for other time zones specified in the input, the result would still be UTC? For example:
|
Yes. The timezone is stripped for large years. So far, there was simply no timezone then. Now, it's |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/util/DateYearDuration.cpp:74
- The comment claims this is "consistent with the export of dates within the range", but
Date::toStringAndType()preserves and re-exports the original timezone/offset (it doesn’t canonicalize toZ). Consider rewording to avoid implying that in-range exports are always UTC/canonicalZ.
// NOTE: For years outside the range of the `Date` class, only the year
// is stored, so the original timezone is not available. Export the
// canonical UTC form (with the `Z`), consistent with the export of
// dates within the range (where a `Date` stores its timezone).
return impl(F{"%d-01-01T00:00:00Z"}, XSD_DATETIME_TYPE);
test/DateYearDurationTest.cpp:492
- This test comment calls the export "canonical UTC form"; more precisely, the large-year representation always exports with the
Z(UTC) designator because the original timezone isn’t stored (including the case where the input had no timezone). Rewording would make the intent clearer.
// NOTE: Only the year is stored, so the export is always in the canonical
// UTC form (with the `Z`), also when the input has no timezone.
testLargeYearDatetime("2039481726-01-01T00:00:00Z", 2039481726);
Overview
Conformance check passed ✅No test result changes. |
|
A date with a year outside
[-9999, 9999]was exported without the timezone designator, while dates within that range keep their timezone. For example, the value"-11700-01-01T00:00:00Z"^^xsd:dateTime(the beginning of the Holocene, from Wikidata) came back from a query as"-11700-01-01T00:00:00"^^xsd:dateTime, which is a different literal than the one that was inserted (anxsd:dateTimewithout a timezone denotes local time). The cutoff is exactly the year range of theDateclass:"-9999-01-01T00:00:00Z"is exported unchanged,"-10000-01-01T00:00:00Z"loses theZ.The reason is that for years outside the range of the
Dateclass,DateYearOrDurationstores only the year (all payload bits are used by the year, so the timezone cannot be stored without shrinking the year range, which would change the meaning of the bits in existing indexes). This change exports the canonical UTC form (with theZ) for such values, which is consistent with the export of dates within the range and restores the round trip for all data whose times are in UTC (in particular, all of Wikidata).NOTE: This was found by the new
qlever check-sync-with-wikidatacommand (qlever-dev/qlever-control#308) on a random sample of entities: the start time of the Holocene onQ18092102was the only difference between the endpoint andSpecial:EntityDatafor that entity.