ponyorm / pony

Pony Object Relational Mapper
Apache License 2.0
3.66k stars 245 forks source link

Datetime doesn't support timezones #434

Open Elijas opened 5 years ago

Elijas commented 5 years ago

Test code:

from datetime import datetime, timezone, timedelta

from pony.orm import *

db = Database()

class Order(db.Entity):
    id = PrimaryKey(int, auto=True)
    time = Required(datetime)

db.bind(provider='sqlite', filename=':memory:')
db.generate_mapping(create_tables=True)

with db_session:
    t1 = datetime.now()
    Order(id=1, time=t1)
    if Order.get(lambda so: so.id == 1) is not None:
        print("Works!")

    t2 = datetime.now(timezone(timedelta(seconds=0)))
    Order(id=2, time=t2)
    if Order.get(lambda so: so.id == 2) is not None:
        print("This is never printed, exception thrown")

Output:

...
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\orm\core.py", line 3949, in get
    if args: return entity._query_from_args_(args, kwargs, frame_depth=cut_traceback_depth+1).get()
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\orm\core.py", line 5874, in get
    objects = query[:2]
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\orm\core.py", line 6130, in __getitem__
    return query._fetch(limit=stop-start, offset=start)
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\orm\core.py", line 6132, in _fetch
    return QueryResult(query, limit, offset, lazy=lazy)
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\orm\core.py", line 6239, in __init__
    self._items = None if lazy else self._query._actual_fetch(limit, offset)
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\orm\core.py", line 5777, in _actual_fetch
    used_attrs=translator.get_used_attrs())
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\orm\core.py", line 4247, in _fetch_objects
    obj._db_set_(avdict)
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\orm\core.py", line 4871, in _db_set_
    % (obj.__class__.__name__, attr.name, obj, old_dbval, new_dbval))
  File "C:\Users\user\.virtualenvs\pony-orm-bug-test\lib\site-packages\pony\utils\utils.py", line 106, in throw
    raise exc
pony.orm.core.UnrepeatableReadError: Value of Order.time for Order[2] was updated outside of current transaction (was: datetime.datetime(2019, 3, 13, 17, 54, 28, 584688, tzinfo=datetime.timezone.utc), now: datetime.datetime(2019, 3, 13, 17, 54, 28, 584688))

Process finished with exit code 1

Happens for both, SQLite PostgreSQL, Pony v0.7.9. I'd appreciate if at least PonyORM would throw an Exception that timezones are unsupported at the moment, instead of just crumbling down, with no apparent reason.

EDIT: Once I was able to isolate the cause - timezones, I've found an open unanswered PR from two years ago :( https://github.com/ponyorm/pony/issues/241

EDIT 2: Made a quickfix, this will remove the timezone information and make the timezone UTC. This is not wise to do, but in my case will be sufficient.

def make_naive_utc(t):
    utc_timezone = timezone(timedelta(seconds=0))
    return t.astimezone(utc_timezone).replace(tzinfo=None)

Alternative solution is here: https://eureka.ykyuen.info/2015/04/17/python-convert-a-timezone-aware-datetime-to-utc-datetime-using-pytz/

craftyguy commented 5 years ago

@Elijas were you able to implement your workaround in a hybrid method/property in the entity? Seems like there might be an unrelated bug with accessing external modules/methods from within a hybrid method (everything is an 'invalid type').. but I may not be doing it correctly.

Elijas commented 5 years ago

@craftyguy Disclaimer, just my opinion ahead, I'm not a dev or Pony pro and most likely will be wrong :)

Pony works by "hijacking" python code from the interpreter itself in order to translate it to SQL. The limitation is that only a limited subset of python works, and only in a limited way (For example, if I remember correctly, I could not move a lambda function out of a query to an external variable).

Entity methods/properties might be under this trade-off too, so that you'd be able to use your custom made Entity properties in queries, for example. Entity properties are limited to one python code line too.

https://stackoverflow.com/questions/51879711/python-pony-orm-one-line-property-inside-declarative-queries

I'd recommend either moving the functionality to other class/function. Or, you could be using a wrapper class, with the additional methods added here, because here they won't be picked up by the Pony's translation-to-SQL and you'll still be able to use all of the other Entity properties/functions like normal.

class EntityWrapper(object):
    '''
    Object wrapper class.
    This a wrapper for objects. It is initialiesed with the object to wrap
    and then proxies the unhandled getattribute methods to it.
    Other classes are to inherit from it.
    '''
    def __init__(self, obj):
        '''
        Wrapper constructor.
        @param obj: object to wrap
        '''
        # wrap the object
        self._wrapped_obj = obj

    def __getattr__(self, attr):
        # see if this object has attr
        # NOTE do not use hasattr, it goes into
        # infinite recurrsion
        if attr in self.__dict__:
            # this object has it
            return getattr(self, attr)
        # proxy to the wrapped object
        return getattr(self._wrapped_obj, attr)

source

craftyguy commented 5 years ago

@Elijas

Thanks for the response/details, I appreciate it. Yet this is the exact issue I hit when trying to use a 'hybrid method' (interestingly enough, sqlalchemy's hybrid_method stuff seems to provide exactly what I want?).

I was hoping to avoid wrapping entities, but I guess I have no choice until this issue is resolved.

jamie--stewart commented 5 years ago

EDIT I may have spoken too soon, I was just testing with UTC. Testing with a different timezone is now failing (the timezone isn't reaching the DB), though my code doesn't fall over in the same way that you describe in the original post any more


Hey both, I ran into this exact same issue earlier, and found another possible workaround:

In my Pony Entity class I've updated my attribute like so:

price_timestamp = Required(datetime, sql_type='TIMESTAMP WITH TIME ZONE')

☝️ @kozlovsky's comment in #9 points out that you can add a sql_type to your attribute definition. Docs reference here too

Doing that, and also migrating my column in the DB to a TIMESTAMP WITH TIME ZONE has fixed my issue (which was creating an entity with the timezone stored in the first place, and then my code thinking that field required an update because it was comparing unaware/aware datetimes)

I know that you cite SQLite in your original issue, and I'm using PostgreSQL so not sure if it will ultimately be of any use to you. I like your workaround too 👍

Elijas commented 5 years ago

EDIT I may have spoken too soon, I was just testing with UTC. Testing with a different timezone is now failing (the timezone isn't reaching the DB), though my code doesn't fall over in the same way that you describe in the original post any more

Hey both, I ran into this exact same issue earlier, and found another possible workaround:

In my Pony Entity class I've updated my attribute like so:

price_timestamp = Required(datetime, sql_type='TIMESTAMP WITH TIME ZONE')

☝️ @kozlovsky's comment in #9 points out that you can add a sql_type to your attribute definition. Docs reference here too

Doing that, and also migrating my column in the DB to a TIMESTAMP WITH TIME ZONE has fixed my issue (which was creating an entity with the timezone stored in the first place, and then my code thinking that field required an update because it was comparing unaware/aware datetimes)

I know that you cite SQLite in your original issue, and I'm using PostgreSQL so not sure if it will ultimately be of any use to you. I like your workaround too 👍

I was using PostgreSQL too, SQLite test was just there to illustrate the point that it isn't an issue with only a certain database back-end (and more importantly, it would be really nice if the error message checked for timezone information and at least crashed with the message that timezones are not supported, as I've spent quite some time figuring out the cause of UnrepeatableReadError @kozlovsky)

Let us know if you'll find a workaround! Great work so far, regardless🙌

lambda-xyz commented 5 years ago

This bug should be fixed because in some situation, PonyORM is not able to give you back a datetime. When the function sql2py() of SQLiteDatetimeConverter is called, it returns str in specific case.

The code below allows to reproduce the bug:

from datetime import datetime, timezone

from pony.orm import Database, Optional, db_session

db = Database('sqlite', 'test.sqlite', create_db=True)

class Product(db.Entity):
    name = Optional(str)
    creation_date = Optional(datetime, volatile=True)

db.generate_mapping(create_tables=True)

with db_session:
    dt = datetime(2019, 8, 12, 17, 26, 3)
    dt_with_ms = datetime(2019, 8, 12, 17, 26, 3, 1332)
    dt_with_tz = datetime(2019, 8, 12, 17, 26, 3, tzinfo=timezone.utc)
    dt_with_ms_and_tz = datetime(2019, 8, 12, 17, 26, 3, 1332, tzinfo=timezone.utc)

    coca = Product(name='coca-cola', creation_date=dt)
    pepsi = Product(name='pepsi', creation_date=dt_with_ms)
    fanta = Product(name='fanta', creation_date=dt_with_tz)
    schweppes = Product(name='schweppes', creation_date=dt_with_ms_and_tz)

    print(coca.to_dict())
    print(pepsi.to_dict())
    print(fanta.to_dict())
    print(schweppes.to_dict())

    db.Product.select()[:].show(500)

It will output:

{'id': 1, 'name': 'coca-cola', 'creation_date': datetime.datetime(2019, 8, 12, 17, 26, 3)}
{'id': 2, 'name': 'pepsi', 'creation_date': datetime.datetime(2019, 8, 12, 17, 26, 3, 1332)}
{'id': 3, 'name': 'fanta', 'creation_date': '2019-08-12 17:26:03+00:00'}
{'id': 4, 'name': 'schweppes', 'creation_date': datetime.datetime(2019, 8, 12, 17, 26, 3, 1332)}

I think a quick fix could be to write datetime2timestamp and timestamp2datetime like this:

def datetime2timestamp(d):
    # unfortunately d.isoformat(' ', timespec='microseconds') works only with python>=3.6
    return d.strftime("%Y-%m-%d %H:%M:%S.%f%z")  # always write microseconds and timezone if any

def timestamp2datetime(t):
    try:
        return datetime.strptime(t, '%Y-%m-%d %H:%M:%S.%f%z')  # timezone
    except ValueError:
        return datetime.strptime(t, '%Y-%m-%d %H:%M:%S.%f')  # no timezone
rokclimb15 commented 4 years ago

In MySQL prior to 8.0.19 it is very common to convert dates with timezones to UTC for storage which makes the quick fix in https://github.com/ponyorm/pony/issues/434#issue-420632554 a decent solution. It doesn't very elegantly handle conversion/display when this value is retrieved again, which is available in 8.0.19.

Would anyone be interested in a patch to enable TZ storage of datetime if the server is new enough to support it?

nm17 commented 3 years ago

Any ETA on this? This is a very important issue.

fluffy-critter commented 3 years ago

This bug should be fixed because in some situation, PonyORM is not able to give you back a datetime. When the function sql2py() of SQLiteDatetimeConverter is called, it returns str in specific case.

I'm currently running into this as well and it's making one of my projects have a really annoying time, where I'm unable to update properties on an object; the ORM fails to update the whole object because it's trying to call str.isoformat even though I never even touched the property that's being incorrectly round-tripped (and anyway, it's unfortunate that the property is coming back as a str instead of a datetime).

AstraLuma commented 2 years ago

Just hit this when I tried to determine how old a record is and I used timezones like a good developer.

j4hangir commented 1 year ago

This one just came to my attention when doing a datetime comparison using lambdas. All date in the database are stored in UTC, and the given datetime in a timezone-aware datetime, yet the comparison ignores it and hence some weird bugs.

j4hangir commented 6 months ago

I have this weird bug, when I do this:

Attribute:

from datetime import datetime as DT

....
    edited: DT = Optional(DT, nullable=True)

Tx:

ret.edited = DT.now()

That works, but this throws the following error:

ret.edited = DT.now(datetime.UTC)

Value of MessageText.edited for MessageText[1227] was updated outside of current transaction (was: datetime.datetime(2024, 5, 27, 0, 55, 41, tzinfo=datetime.timezone.utc), now: datetime.datetime(2024, 5, 27, 0, 55, 41))

The edited is of course never set or modified elsewhere and visibly something goes off when datetime has UTC timezone.