Support TzDate, TzDatetime and TzTimestamp types#857
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #857 +/- ##
==========================================
+ Coverage 81.23% 81.30% +0.06%
==========================================
Files 94 94
Lines 12121 12165 +44
Branches 1186 1194 +8
==========================================
+ Hits 9847 9891 +44
Misses 1809 1809
Partials 465 465
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Analysis performed by claude, claude-opus-4-6. |
There was a problem hiding this comment.
Pull request overview
Adds full SDK support for YDB’s timezone-carrying primitive types (TzDate, TzDatetime, TzTimestamp) by registering them in PrimitiveType and providing Python ↔ wire conversions, plus tests and documentation updates to prevent the AttributeError crash reported in #405.
Changes:
- Register
TzDate/TzDatetime/TzTimestampinydb.types.PrimitiveTypeand implement text-based (de)serialization usingzoneinfo. - Add unit tests for both query and async table clients covering raw-string and native-datetime modes.
- Document the new types and add a user-facing changelog entry.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
ydb/types.py |
Adds tz parsing/formatting helpers and registers tz primitive types in PrimitiveType. |
tests/query/test_types.py |
Extends query type round-trip tests for tz types (string + native datetime). |
tests/aio/test_types.py |
Extends async table client tests for tz types. |
docs/types.rst |
Documents tz types mapping and result-set behavior. |
CHANGELOG.md |
Adds a user-facing entry for tz type support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3f14e94 to
2f5b90f
Compare
Register the timezone-carrying primitive types so query results and parameters map to/from tz-aware datetime via zoneinfo (raw string when native result-set conversion is disabled). Reading them previously crashed with AttributeError.
79a3160 to
51d8063
Compare
There was a problem hiding this comment.
AI Review Summary
Verdict: ✅ No critical issues found
Critical issues
No critical issues found.
Other findings
- Minor | Medium:
_parse_tzexception handling —ZoneInfo()can raise exceptions beyondZoneInfoNotFoundErrorfor certain malformed keys (e.g. empty string raisesValueErroron CPython). While unlikely with valid YDB wire data, this bypasses the graceful fallback. —ydb/types.py:95 - Minor | Low: TZ types reuse the existing non-TZ native-mode settings (
_native_date_in_result_sets, etc.) rather than having dedicated toggles. Users cannot independently control native conversion for e.g.DatevsTzDate. —ydb/types.py:107 - Nit | Low:
_to_tz_timestampalways writes 6-digit microseconds viastrftime("%f"), producing.000000when microseconds are zero. If YDB normalizes the wire representation (stripping trailing zeros), a write→read→write cycle could produce different strings for the same value. —ydb/types.py:140 - Nit | Low: No test covers TzDate with non-midnight time components (lossy truncation) or TzTimestamp with zero microseconds (format difference). These edge cases would help document expected behavior. —
tests/query/test_types.py
This review was generated automatically. Critical issues require attention; other findings are advisory.
If this comment was useful, please give it a 👍 — it helps us improve the review bot.
|
Analysis performed by claude, claude-opus-4-6. |
… except, canonical TzTimestamp microseconds
Fixes #405.
TzDate,TzDatetimeandTzTimestampwere not registered inPrimitiveType, so reading them crashed withAttributeError: 'NoneType' object has no attribute 'get_value'. This registers all three (stored on the wire as"<value>,<zone>"text):datetimecarrying azoneinfo.ZoneInfo(TzDateat midnight); the raw"<value>,<zone>"string when the matchingwith_native_*_in_result_setsoption is off, mirroring the other date/time types.datetime(its local wall-clock +ZoneInfoname is stored) or the raw string.Based on
drop-python-3.9-and-proto3for the Python 3.10 baseline (stdlibzoneinfo); retarget tomainonce that merges.