Repository navigation
Accept the Z designator in timestamptz strings - #4015
anishmehta24 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Adds support for trailing Z UTC designators in timestamptz parsing.
Changes:
- Accept
Zin microsecond and nanosecond timestamp formats. - Normalize
Zto+00:00for Python 3.10 compatibility. - Add conversion tests for
Ztimestamps.
| File | Description |
|---|---|
tests/utils/test_datetime.py |
Tests Z-terminated timestamptz conversions. |
pyiceberg/utils/datetime.py |
Supports and normalizes Z timestamps. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ISO_TIMESTAMPTZ = re.compile(r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d{1,6})?[-+]\d{2}:\d{2}") | ||
| ISO_TIMESTAMPTZ_NANO = re.compile(r"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2})(.\d{1,6})?(\d{1,3})?([-+]\d{2}:\d{2})") | ||
| ISO_TIMESTAMPTZ = re.compile(r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d{1,6})?([-+]\d{2}:\d{2}|Z)") | ||
| ISO_TIMESTAMPTZ_NANO = re.compile(r"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2})(.\d{1,6})?(\d{1,3})?([-+]\d{2}:\d{2}|Z)") |
There was a problem hiding this comment.
It looks like Java also supports lowercase z (and no seconds). Could you add those while you're here?
There was a problem hiding this comment.
Done, both timestamptz patterns now accept a lowercase z and leave out the seconds (a fraction still needs seconds before it, like in Java). Added cases for both to the tests.
|
@rambleraptor added the lowercase z and no-seconds forms, could you take another look? |
|
@Fokko could you take a look when you have time? The review comments are addressed. |
Rationale for this change
Timestamptz strings that end in
Zare rejected, even thoughZis the ISO-8601 designator for UTC and Java'sOffsetDateTime.parseaccepts it:The same filter with
+00:00works.ISO_TIMESTAMPTZandISO_TIMESTAMPTZ_NANOnow acceptZas well as+HH:MM/-HH:MM.Zis swapped for+00:00before callingdatetime.fromisoformat, because on Python 3.10fromisoformatdoesn't acceptZ.Are these changes tested?
Yes. There are new cases in
tests/utils/test_datetime.pyfortimestamptz_to_microsandtimestamptz_to_nanoswithZ, and they fail without the change. The datetime, literal, transform and conversion tests pass (735). I also checked theZcase on Python 3.10.Are there any user-facing changes?
Yes. Timestamptz literals and row filters now accept a trailing
Z.