Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timestamptz autoconversion #978

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
8cfa0c7
Added column tz autoconversion to Table __init__ method
aabmets Mar 17, 2024
95d8e4d
Added tz ability to Timestamptz
aabmets Mar 18, 2024
7dd76d8
Added tz ability to TimestamptzNow
aabmets Mar 18, 2024
06e0625
Added timezone awareness to TimestamptzOffset and TimestamptzCustom
aabmets Mar 19, 2024
c589d44
Added zoneinfo fallback dependency for missing zone data
aabmets Mar 19, 2024
fc708fc
Added backports.zoneinfo
aabmets Mar 19, 2024
705629a
Un-privatized tz attribute from default timestamptz classes
aabmets Mar 19, 2024
a25e814
Added python version constraint to backports.zoneinfo
aabmets Mar 19, 2024
86004b0
Merge branch 'piccolo-orm:master' into timestamptz-autoconversion
aabmets Mar 20, 2024
6611a97
Updated Timestamptz docstring and fixed a bug in TimestamptzCustom.fr…
aabmets Mar 20, 2024
aa9f077
Fixed bug in TimestamptzCustom.from_datetime
aabmets Mar 20, 2024
6fcb93f
Fixed linter and test issues (hopefully)
aabmets Mar 26, 2024
59a531e
Added backport.zoneinfo import with try-except clause, fixed imports …
aabmets Mar 28, 2024
7a3c584
Fixed linting errors across codebase and fixed timestamptz test error…
aabmets Mar 28, 2024
41a6d0c
Fixed zoneinfo module import for autogenerated migrations files
aabmets Mar 28, 2024
ad0d1d1
I swear to god if this commit doesn't fix the linting and test errors
aabmets Mar 28, 2024
c816d8e
Added more ZoneInfo import ignore rules for MyPy
aabmets Mar 28, 2024
cbf6d41
Added pragma no cover to ZoneInfo import except clauses
aabmets Mar 28, 2024
db53939
Removed ZoneInfo import from table.py
aabmets Mar 29, 2024
de33a9b
Merge branch 'master' into timezonetz-autoconversion
dantownsend Apr 5, 2024
36b0da6
move `tz` after `default` for better backwards compatibility
dantownsend Apr 5, 2024
9fda29d
drop `tz_type` - I don't think we need it for now
dantownsend Apr 5, 2024
3733218
add missing continues
dantownsend Apr 5, 2024
7c6ce55
add `at_timezone` clause
dantownsend Apr 5, 2024
755b059
remove unused imports
dantownsend Apr 5, 2024
026b182
check `!= ZoneInfo("UTC")`
dantownsend Apr 5, 2024
e9b02be
centralised imports for ZoneInfo
dantownsend Apr 5, 2024
4327178
added method for getting column alias
dantownsend Apr 5, 2024
e9fae63
make sure all Timestamptz values have a tz set
dantownsend Apr 5, 2024
4416adc
fix typos
dantownsend Apr 5, 2024
0ab2d24
fix tests
dantownsend Apr 5, 2024
c64efa9
fix sqlite
dantownsend Apr 5, 2024
1d98c36
refactor `objects` queries to proxy to `select`
dantownsend Apr 6, 2024
2c75e70
rename `tz` to `at_time_zone`
dantownsend Apr 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion piccolo/apps/migrations/auto/serialisation.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,17 @@
from dataclasses import dataclass, field
from enum import Enum

from piccolo.columns import Column
from piccolo.columns import Column, Timestamptz
from piccolo.columns.defaults.base import Default
from piccolo.columns.defaults.timestamptz import (
TimestamptzCustom,
TimestamptzNow,
TimestamptzOffset,
)
from piccolo.columns.reference import LazyTableReference
from piccolo.table import Table
from piccolo.utils.repr import repr_class_instance
from piccolo.utils.zoneinfo import ZoneInfo

from .serialisation_legacy import deserialise_legacy_params

Expand Down Expand Up @@ -546,6 +552,30 @@ def serialise_params(params: t.Dict[str, t.Any]) -> SerialisedParams:
expect_conflict_with_global_name=UniqueGlobalNames.DEFAULT,
)
)
# ZoneInfo for Timestamptz* instances
in_group = (
Timestamptz,
TimestamptzNow,
TimestamptzCustom,
TimestamptzOffset,
)
if isinstance(value, in_group):
extra_imports.append(
Import(
module=ZoneInfo.__module__,
target=None,
)
)
continue

# ZoneInfo instances
if isinstance(value, ZoneInfo):
extra_imports.append(
Import(
module=value.__class__.__module__,
target=None,
)
)
continue

# Dates and times
Expand Down Expand Up @@ -633,6 +663,7 @@ def serialise_params(params: t.Dict[str, t.Any]) -> SerialisedParams:
extra_imports.append(
Import(module=module_name, target=type_.__name__)
)
continue

# Functions
if inspect.isfunction(value):
Expand Down
10 changes: 8 additions & 2 deletions piccolo/columns/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,9 @@ def as_alias(self, name: str) -> Column:
column._alias = name
return column

def _get_alias(self) -> str:
return self._alias or self._meta.get_default_alias()

def join_on(self, column: Column) -> ForeignKey:
"""
Joins are typically performed via foreign key columns. For example,
Expand Down Expand Up @@ -945,8 +948,8 @@ def ddl(self) -> str:

return query

def copy(self) -> Column:
column: Column = copy.copy(self)
def copy(self: Self) -> Self:
column = copy.copy(self)
column._meta = self._meta.copy()
return column

Expand All @@ -971,3 +974,6 @@ def __repr__(self):
f"{table_class_name}.{self._meta.name} - "
f"{self.__class__.__name__}"
)


Self = t.TypeVar("Self", bound=Column)
75 changes: 58 additions & 17 deletions piccolo/columns/column_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class Band(Table):
from piccolo.querystring import QueryString, Unquoted
from piccolo.utils.encoding import dump_json
from piccolo.utils.warnings import colored_warning
from piccolo.utils.zoneinfo import ZoneInfo

if t.TYPE_CHECKING: # pragma: no cover
from piccolo.columns.base import ColumnMeta
Expand Down Expand Up @@ -955,30 +956,33 @@ def __set__(self, obj, value: t.Union[datetime, None]):
class Timestamptz(Column):
"""
Used for storing timezone aware datetimes. Uses the ``datetime`` type for
values. The values are converted to UTC in the database, and are also
returned as UTC.
values. The values are converted to UTC when saved into the database and
are converted back into the timezone of the column on select queries.

**Example**

.. code-block:: python

import datetime
from zoneinfo import ZoneInfo

class Concert(Table):
starts = Timestamptz()
class TallinnConcerts(Table):
event_start = Timestamptz(at_time_zone=ZoneInfo("Europe/Tallinn"))

# Create
>>> await Concert(
... starts=datetime.datetime(
... year=2050, month=1, day=1, tzinfo=datetime.timezone.tz
Comment on lines -971 to -973
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that tzinfo=datetime.timezone.tz should be replaced with tzinfo=datetime.timezone.utc in the old documentation.

>>> await TallinnConcerts(
... event_start=datetime.datetime(
... year=2050, month=1, day=1, hour=20
... )
... ).save()

# Query
>>> await Concert.select(Concert.starts)
>>> await TallinnConcerts.select(TallinnConcerts.event_start)
{
'starts': datetime.datetime(
2050, 1, 1, 0, 0, tzinfo=datetime.timezone.utc
'event_start': datetime.datetime(
2050, 1, 1, 20, 0, tzinfo=zoneinfo.ZoneInfo(
key='Europe/Tallinn'
)
)
}

Expand All @@ -993,22 +997,59 @@ class Concert(Table):
timedelta_delegate = TimedeltaDelegate()

def __init__(
self, default: TimestamptzArg = TimestamptzNow(), **kwargs
self,
default: TimestamptzArg = TimestamptzNow(),
at_time_zone: ZoneInfo = ZoneInfo("UTC"),
**kwargs,
) -> None:
self._validate_default(
default, TimestamptzArg.__args__ # type: ignore
)

if isinstance(default, datetime):
default = TimestamptzCustom.from_datetime(default)
default = TimestamptzCustom.from_datetime(default, at_time_zone)

if default == datetime.now:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about this if statement. It seems that datetime.now is a callable, which doesn't seem to align with the TimestamptzArg from the signature. By the way, it seems that the test doesn't cover this scenario involving a callable.

Copy link
Contributor

@jrycw jrycw Apr 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, it should be the line if default == datetime.now:.

default = TimestamptzNow()
default = TimestamptzNow(tz=at_time_zone)

self._at_time_zone = at_time_zone
self.default = default
kwargs.update({"default": default})
kwargs.update({"default": default, "at_time_zone": at_time_zone})
super().__init__(**kwargs)

###########################################################################

def at_time_zone(self, time_zone: t.Union[ZoneInfo, str]) -> Timestamptz:
"""
By default, the database returns the value in UTC. This lets us get
the value converted to the specified timezone.
"""
time_zone = (
ZoneInfo(time_zone) if isinstance(time_zone, str) else time_zone
)
instance = self.copy()
instance._at_time_zone = time_zone
return instance
Comment on lines +1022 to +1032
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding #959, it appears that we are creating a new object by copying self and then modifying the _at_time_zone attribute of that object to match the time_zone variable, without altering self. Is my understanding correct?


###########################################################################

def get_select_string(
self, engine_type: str, with_alias: bool = True
) -> str:
select_string = self._meta.get_full_name(with_alias=False)

if self._at_time_zone != ZoneInfo("UTC"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be other instances where timezone comparisons are performed similarly, and I'll use this one as an example. It seems this type of comparison could lead to unexpected outcomes. The following example might illustrate this unpredictability.

>>> import datetime
>>> from zoneinfo import ZoneInfo
>>> tz1 = datetime.timezone.utc
>>> tz2 = ZoneInfo("UTC")
>>> d1 = datetime.datetime(year=2024, month=4, day=7, tzinfo=tz1)
>>> d2 = datetime.datetime(year=2024, month=4, day=7, tzinfo=tz2)
>>> d1 == d2
True
>>> str(d1) == str(d2)
True
>>> d1.tzinfo, d2.tzinfo
(datetime.timezone.utc, zoneinfo.ZoneInfo(key='UTC'))
>>> d1.tzinfo == d2.tzinfo
False
>>> d1.tzinfo is d2.tzinfo
False

# SQLite doesn't support `AT TIME ZONE`, so we have to do it in
# Python instead (see ``Select.response_handler``).
if self._meta.engine_type in ("postgres", "cockroach"):
Copy link
Contributor

@jrycw jrycw Apr 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Occurrences of in or == checks like if self._meta.engine_type in ("postgres", "cockroach"): seem to be becoming increasingly common. I would recommend using data structures like Enum to mitigate typos and rely more on auto-completion while coding.

select_string += f" AT TIME ZONE '{self._at_time_zone.key}'"

if with_alias:
alias = self._get_alias()
select_string += f' AS "{alias}"'

return select_string

###########################################################################
# For update queries

Expand Down Expand Up @@ -2305,7 +2346,7 @@ def arrow(self, key: str) -> JSONB:
Allows part of the JSON structure to be returned - for example,
for {"a": 1}, and a key value of "a", then 1 will be returned.
"""
instance = t.cast(JSONB, self.copy())
instance = self.copy()
instance.json_operator = f"-> '{key}'"
return instance

Expand All @@ -2318,7 +2359,7 @@ def get_select_string(
select_string += f" {self.json_operator}"

if with_alias:
alias = self._alias or self._meta.get_default_alias()
alias = self._get_alias()
select_string += f' AS "{alias}"'

return select_string
Expand Down Expand Up @@ -2623,7 +2664,7 @@ def get_select_string(self, engine_type: str, with_alias=True) -> str:
select_string += f"[{self.index}]"

if with_alias:
alias = self._alias or self._meta.get_default_alias()
alias = self._get_alias()
select_string += f' AS "{alias}"'

return select_string
Expand Down
58 changes: 48 additions & 10 deletions piccolo/columns/defaults/timestamptz.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,28 @@
from __future__ import annotations

import datetime
import datetime as pydatetime
import typing as t
from enum import Enum

from piccolo.utils.zoneinfo import ZoneInfo

from .timestamp import TimestampCustom, TimestampNow, TimestampOffset


class TimestamptzOffset(TimestampOffset):
def __init__(
self,
days: int = 0,
hours: int = 0,
minutes: int = 0,
seconds: int = 0,
tz: ZoneInfo = ZoneInfo("UTC"),
):
self.tz = tz
super().__init__(
days=days, hours=hours, minutes=minutes, seconds=seconds
)

@property
def cockroach(self):
interval_string = self.get_postgres_interval_string(
Expand All @@ -16,9 +31,7 @@ def cockroach(self):
return f"CURRENT_TIMESTAMP + INTERVAL '{interval_string}'"

def python(self):
return datetime.datetime.now(
tz=datetime.timezone.utc
) + datetime.timedelta(
return pydatetime.datetime.now(tz=self.tz) + pydatetime.timedelta(
days=self.days,
hours=self.hours,
minutes=self.minutes,
Expand All @@ -27,35 +40,60 @@ def python(self):


class TimestamptzNow(TimestampNow):
def __init__(self, tz: ZoneInfo = ZoneInfo("UTC")):
self.tz = tz

@property
def cockroach(self):
return "current_timestamp"

def python(self):
return datetime.datetime.now(tz=datetime.timezone.utc)
return pydatetime.datetime.now(tz=self.tz)


class TimestamptzCustom(TimestampCustom):
def __init__(
self,
year: int = 2000,
month: int = 1,
day: int = 1,
hour: int = 0,
second: int = 0,
microsecond: int = 0,
tz: ZoneInfo = ZoneInfo("UTC"),
):
self.tz = tz
super().__init__(
year=year,
month=month,
day=day,
hour=hour,
second=second,
microsecond=microsecond,
)

@property
def cockroach(self):
return "'{}'".format(self.datetime.isoformat().replace("T", " "))

@property
def datetime(self):
return datetime.datetime(
return pydatetime.datetime(
year=self.year,
month=self.month,
day=self.day,
hour=self.hour,
second=self.second,
microsecond=self.microsecond,
tzinfo=datetime.timezone.utc,
tzinfo=self.tz,
)

@classmethod
def from_datetime(cls, instance: datetime.datetime): # type: ignore
def from_datetime(
cls, instance: pydatetime.datetime, tz: ZoneInfo = ZoneInfo("UTC")
): # type: ignore
if instance.tzinfo is not None:
instance = instance.astimezone(datetime.timezone.utc)
instance = instance.astimezone(tz)
return cls(
year=instance.year,
month=instance.month,
Expand All @@ -72,7 +110,7 @@ def from_datetime(cls, instance: datetime.datetime): # type: ignore
TimestamptzOffset,
Enum,
None,
datetime.datetime,
pydatetime.datetime,
]


Expand Down
Loading
Loading