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

Large interval not returned correctly as timedelta #512

Closed
mfenniak opened this Issue Feb 23, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@mfenniak
Copy link

mfenniak commented Feb 23, 2017

Python 3.6.0 (default, Dec 24 2016, 00:01:50)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import psycopg2
>>> conn = psycopg2.connect()
>>> cursor = conn.cursor()
>>> cursor.execute("SELECT '999999:00:00'::interval")
>>> cursor.fetchone()
(datetime.timedelta(-24856, 74752),)
>>> import datetime
>>> datetime.timedelta(hours=999999)
datetime.timedelta(41666, 54000)

The PG interval "999999:00:00" gets returned from psycopg2 as datetime.timedelta(-24856, 74752), when I would expect it to be returned as datetime.timedelta(41666, 54000). The timedelta type supports this precision of time, so it seems like this is an overflow inside psycopg2.

psycopg2==2.6.2

@dvarrazzo

This comment has been minimized.

Copy link
Member

dvarrazzo commented Feb 24, 2017

You are right, there was an integer overflow indeed.

While I could fix it no problem on Linux, weirdly the problem doesn't disappear on Windows. I rewrote the parsing algorithm to do without almost any floating point operation and only using longs... and this is still baffling me. Could it be the case that on Windows the Python interval is buggy? @jerickso, do you get these results on Windows? If so what could be the difference?

>>> timedelta(hours=999999)
datetime.timedelta(41666, 54000)

>>> timedelta(seconds=999999*60*60)
datetime.timedelta(41666, 54000)
@jerickso

This comment has been minimized.

Copy link
Member

jerickso commented Feb 24, 2017

Unfortunately it doesn't seem to be Python on windows:

Python 2.7.11 (v2.7.11:6d1b6a68f775, Dec  5 2015, 20:40:30) [MSC v.1500 64 bit (AMD64)] on win32
>>> import datetime
>>> datetime.timedelta(hours=999999)
datetime.timedelta(41666, 54000)
>>> datetime.timedelta(seconds=999999*60*60)
datetime.timedelta(41666, 54000)

And it is not the assert compare (even tho it is returning a float instead of an int)

>>> t = datetime.timedelta(seconds=999999*60*60)
>>> t.days*24 + t.seconds/60.0/60
999999.0
>>> z = _
>>> assert z == 999999.0
>>> assert z == 999999
>>>

Looking at the error message closer, -694970896 is the signed two's complement representation of 3599996400. So it is a size issue.

MS's 'long' type is the same size as an 'int' type, regardless if compiling for 32bit or 64bit.

We could use 'long long' with the python format value of 'L', or if the size needs to change based upon the platform, we've used Py_ssize_t in the past.

@dvarrazzo dvarrazzo closed this in 14fe3ad Feb 24, 2017

@dvarrazzo

This comment has been minimized.

Copy link
Member

dvarrazzo commented Feb 24, 2017

That makes sense, @jerickso, thank you very much!

@mfenniak

This comment has been minimized.

Copy link

mfenniak commented Feb 25, 2017

@dvarrazzo Thank you for the very responsive patch for this issue. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment