Skip to content

sqlite3 timestamp adapter chokes on timezones #63265

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

Closed
acdha mannequin opened this issue Sep 22, 2013 · 10 comments
Closed

sqlite3 timestamp adapter chokes on timezones #63265

acdha mannequin opened this issue Sep 22, 2013 · 10 comments
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@acdha
Copy link
Mannequin

acdha mannequin commented Sep 22, 2013

BPO 19065
Nosy @malemburg, @abalkin, @pitrou, @bitdancer, @berkerpeksag, @vajrasky, @iafisher
Files
  • add_timezone_support_for_sqlite3_datetime_adapter.patch: Initial patch by Chris Adams (acdha). Added support for negative timezone, doc, unit test and coding nitpick.
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2016-03-27.17:00:12.736>
    created_at = <Date 2013-09-22.00:03:21.434>
    labels = ['easy', 'type-feature', 'library']
    title = 'sqlite3 timestamp adapter chokes on timezones'
    updated_at = <Date 2021-10-07.11:07:17.431>
    user = 'https://bugs.python.org/acdha'

    bugs.python.org fields:

    activity = <Date 2021-10-07.11:07:17.431>
    actor = 'iafisher'
    assignee = 'ghaering'
    closed = True
    closed_date = <Date 2016-03-27.17:00:12.736>
    closer = 'berker.peksag'
    components = ['Library (Lib)']
    creation = <Date 2013-09-22.00:03:21.434>
    creator = 'acdha'
    dependencies = []
    files = ['32253']
    hgrepos = []
    issue_num = 19065
    keywords = ['patch', 'easy']
    message_count = 10.0
    messages = ['198234', '198269', '198288', '200590', '245836', '248826', '248834', '249009', '254314', '262526']
    nosy_count = 10.0
    nosy_names = ['lemburg', 'ghaering', 'belopolsky', 'pitrou', 'r.david.murray', 'berker.peksag', 'acdha', 'vajrasky', 'jacques-piguet', 'iafisher']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue19065'
    versions = ['Python 3.4']

    @acdha
    Copy link
    Mannequin Author

    acdha mannequin commented Sep 22, 2013

    If you use detect_types=sqlite3.PARSE_DECLTYPES with sqlite3 and insert a timezone-aware datetime instance, you will get a ValueError if you attempt to read it back out:

    File "/usr/local/Cellar/python3/3.3.2/Frameworks/Python.framework/Versions/3.3/lib/python3.3/sqlite3/dbapi2.py", line 68, in convert_timestamp
    hours, minutes, seconds = map(int, timepart_full[0].split(b":"))
    ValueError: invalid literal for int() with base 10: '03+00'

    Although this immediately gets into the thorny stdlib timezone support situation, it's extremely annoying to have the out-of-the-box module break round-tripping data and it looks like support for simple UTC offsets isn't too horrible – something like https://gist.github.com/acdha/6655391 works in very limited testing:

    def tz_aware_timestamp_adapter(val):
        datepart, timepart = val.split(b" ")
        year, month, day = map(int, datepart.split(b"-"))
    
        if b"+" in timepart:
            timepart, tz_offset = timepart.rsplit(b"+", 1)
            if tz_offset == b'00:00':
                tzinfo = datetime.timezone.utc
            else:
                hours, minutes = map(int, tz_offset.split(b':', 1))
                tzinfo = datetime.timezone(datetime.timedelta(hours=hours, minutes=minutes))
        else:
            tzinfo = None
    
        timepart_full = timepart.split(b".")
        hours, minutes, seconds = map(int, timepart_full[0].split(b":"))
    
        if len(timepart_full) == 2:
            microseconds = int('{:0<6.6}'.format(timepart_full[1].decode()))
        else:
            microseconds = 0
    
        val = datetime.datetime(year, month, day, hours, minutes, seconds, microseconds, tzinfo)
    
        return val
    
    sqlite3.register_converter('timestamp', tz_aware_timestamp_adapter)

    @acdha acdha mannequin added the stdlib Python modules in the Lib dir label Sep 22, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Sep 22, 2013

    This would be an useful improvement. Do you want to post a patch? See guidelines at http://docs.python.org/devguide/

    @pitrou pitrou added the type-feature A feature request or enhancement label Sep 22, 2013
    @abalkin
    Copy link
    Member

    abalkin commented Sep 22, 2013

    Sounds like a reasonable request, but the proposed code does not seem to work for the Eastern hemisphere (negative tz offsets.)

    I am not very familiar with sqlite module. What timestamp format does it use? Isn't it some varian of ISO 3339? See bpo-15873.

    @abalkin abalkin added the easy label Sep 22, 2013
    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Oct 20, 2013

    Added patch to add timezone support for sqlite3 datetime adapter.

    @jacques-piguet
    Copy link
    Mannequin

    jacques-piguet mannequin commented Jun 26, 2015

    I just hit the problem in Python 2.7.
    The patch given here is not applicable due to missing class "datetime.timezone" in Python before 3.2.

    Question 1: when will the patch be applied in Python 3?
    Question 2: any chance to get a solution in Python 2?

    @jacques-piguet jacques-piguet mannequin added type-crash A hard crash of the interpreter, possibly with a core dump and removed type-feature A feature request or enhancement labels Jun 26, 2015
    @ghaering
    Copy link
    Mannequin

    ghaering mannequin commented Aug 19, 2015

    I'm -1 on adding timezone to the adapters.

    @ghaering ghaering mannequin self-assigned this Aug 19, 2015
    @bitdancer
    Copy link
    Member

    Can you expand on why you are -1, Gerhard?

    @ghaering
    Copy link
    Mannequin

    ghaering mannequin commented Aug 23, 2015

    I'm -1 because I believe that ultimately, adapters and converters were a mistake to add to pysqlite. That's why I deprecated them in pysqlite 2.8.0.

    Do you know what would be the correct step to propose a deprecation in the sqlite3 module of Python proper? Is a PEP necessary?

    @bitdancer
    Copy link
    Member

    Just open an issue to propose the deprecation, if they aren't part of the DBAPI. We wouldn't actually remove them, though, until after 2.7 goes out of maintenance (if then...we still haven't quite decided what we're going to do about removals post 2.7).

    @berkerpeksag
    Copy link
    Member

    Do you know what would be the correct step to propose a deprecation in the sqlite3 module of Python proper?

    I've opened bpo-26651 (patch included) to deprecate them.

    @berkerpeksag berkerpeksag added type-feature A feature request or enhancement and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Mar 27, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants