[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: Xiang Zhang, was there a discussion on Python-Dev? -- ___ Python tracker ___ ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Hassanbot added the comment: This still isn't fixed as of 3.8 (or in master I think). I can understand why you wouldn't want to allow serializing and deserializing time zones, since tzinfo objects cannot be accurately serialized with a simple UTC offset, but you should at least get an error

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: > I'm even worse at that. :-( LoL. Okay I'll do that. :-) -- ___ Python tracker ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: I'm even worse at that. :-( -- ___ Python tracker ___ ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: > Maybe discuss this on Python-Dev? It's fine. Could you compose a mail instead of me? I am not good at that. :-( -- ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: Added few minor comments on Rietveld. Technically the patch LGTM. But the behavior change of the timestamp converter for input without timezone offset perhaps needs wider discussion. We have several options: 0. Current behavior. Silently drop the timezone

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: Ping for review for timestamptz-3.patch. -- ___ Python tracker ___ ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: timestamptz-3.patch uses ValueError instead of ProgrammingError and treat suffix Z as UTC timezone. -- Added file: http://bugs.python.org/file46279/timestamptz-3.patch ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: I am okay with ValueError（actually I use it in the patch originally) but I am not in favour of catching the errors of the parser. The parser raises errors due to invalid data, not timezone. I think propagate it to users could make the reason more obvious.

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: > Perhaps even TypeError should be converted to ValueError. But this is different issue of course. -- ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: I think ProgrammingError is not correct type of an exception. It is used for programming errors such as using uninitialized or closed objects or supplying incorrect number of bindings. But this error can be caused by invalid data in a database created not

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: timestamptz-2.patch make timestamp and timestamptz specilized for their own objects. -- Added file: http://bugs.python.org/file46277/timestamptz-2.patch ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: > I think the old code unlikely will be broken if preserve an exception type > and don't change conditions for raising an error. Currently there could be no error when the object gets a timezone. The timezone is simply discarded and only when microseconds comes

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: Naive and aware objects should not be mixed. Their actually are different types. The converter doesn't return str or bytes when can't parse an input as a datetime, and it shouldn't return a naive datetime if can't find a timezone. SQLite date and time

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: > I think the timestamptz converter should either interpret strings without > timezone as UTC (and perhaps understand the "Z" suffix as sqlite3 date() > function) or raises an error. It should never return naive datetime. Currently timestamptz just convert back

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: I think the timestamptz converter should either interpret strings without timezone as UTC (and perhaps understand the "Z" suffix as sqlite3 date() function) or raises an error. It should never return naive datetime. The timestamp converter needs better

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: timestamptz.patch implements a new converter that's able to convert aware datetime objects. -- stage: needs patch -> patch review Added file: http://bugs.python.org/file46265/timestamptz.patch ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Marc-Andre Lemburg added the comment: On 11.01.2017 17:04, Xiang Zhang wrote: > I am not sure it's worth to make it even optional in 3.7. Discarding tzinfo > sounds weird, and worse, the behaviour is not deterministic, it could also > fail with an exception. Best practice is to store all

[issue29099] sqlite3 timestamp converter cannot handle timezone

Changes by Serhiy Storchaka : -- stage: commit review -> needs patch type: behavior -> enhancement versions: -Python 3.5, Python 3.6 ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Marc-Andre Lemburg added the comment: On 11.01.2017 17:08, Serhiy Storchaka wrote: > > The naive datetime converter is registered under the name "timestamp". The > aware datetime converter or the universal datetime converter (if it is > needed) can be registered under different names. This

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: The naive datetime converter is registered under the name "timestamp". The aware datetime converter or the universal datetime converter (if it is needed) can be registered under different names. -- ___ Python

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: I asked whether this should be treated as an enhancement or bug fix. Now with your concerns I think it fits as enhancement more. > Is it possible to make the support optional for 3.5 and 3.6 and only enable > it for 3.7 (with the possibility of disabling it

[issue29099] sqlite3 timestamp converter cannot handle timezone

Marc-Andre Lemburg added the comment: I don't think this can be considered a bug fix, since it changes behavior for applications that read data from SQLite databases which were not created by Python. Those application may now see datetime values with tz infos and will likely not be prepared

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: Agreed. And maybe raise more informative error for datetimes with a timezone. I'm also not sure about 3.5 and 3.6. Datetimes with timezones were never supported. Currently all returned ditetime objects are naive. Getting an aware datetime can confuse

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: > LGTM except that arguments of assertEqual() should be swapped. Thanks Serhiy! I'll take care of that. One problem remained is what to do with 2.7. There is no timezone object in 2.7. My preference is just pointing out the limitation that only naive datetime

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: LGTM except that arguments of assertEqual() should be swapped. -- stage: patch review -> commit review ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Changes by Xiang Zhang : Added file: http://bugs.python.org/file46254/sqlite3-3.patch ___ Python tracker ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: LGTM generally. :-) -- ___ Python tracker ___ ___ Python-bugs-list mailing

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: In general the patch LGTM. Added comments on Rietveld. -- ___ Python tracker ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Changes by Serhiy Storchaka : -- stage: test needed -> patch review Added file: http://bugs.python.org/file46082/sqlite3-2.patch ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Changes by Bozo Kopic : Added file: http://bugs.python.org/file46081/sqlite3-2.patch ___ Python tracker ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Changes by Bozo Kopic : Removed file: http://bugs.python.org/file46075/sqlite3-2.patch ___ Python tracker ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Changes by Bozo Kopic : Added file: http://bugs.python.org/file46075/sqlite3-2.patch ___ Python tracker ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Changes by Bozo Kopic : Removed file: http://bugs.python.org/file46074/sqlite3-2.patch ___ Python tracker ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Bozo Kopic added the comment: I'm providing new patch that adds support for parsing timezone information. Tests are included. -- Added file: http://bugs.python.org/file46074/sqlite3-2.patch ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: import sqlite3, datetime c = sqlite3.connect(':memory:', detect_types=sqlite3.PARSE_DECLTYPES|sqlite3.PARSE_COLNAMES) cur = c.cursor() cur.execute('create table test(t timestamp)') t = datetime.datetime.now(tz=datetime.timezone.utc) cur.execute("insert into

[issue29099] sqlite3 timestamp converter cannot handle timezone

Serhiy Storchaka added the comment: Could you provide a unittest? -- nosy: +belopolsky, ghaering, lemburg stage: -> test needed ___ Python tracker ___

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: I would prefer add the support for timezone since the current behaviour seems not correct to me. If you'd like to work on it, don't forget to add a test case and sign the CLA. But I am not sure this should be treated as a bug or new feature. -- nosy:

[issue29099] sqlite3 timestamp converter cannot handle timezone

Bozo Kopic added the comment: Yes, that is correct. Timezone information is discarded. Submitted patch only resolves occurrence of ValueError. Is additional patch that enables parsing of timezone required? -- ___ Python tracker

[issue29099] sqlite3 timestamp converter cannot handle timezone

Xiang Zhang added the comment: Thanks for your report Bozo. I think this is due to timestamp converter doesn't handle timezone now. Besides this ValueError, if I understand correctly, even if you pass a datetime object with tzinfo, with microsecond set, the returned datetime object is a wrong