Skip to content

Add variable precision time and timestamp support #229

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.

Sign up for GitHub

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
lpoulain wants to merge 4 commits into from
Closed

Add variable precision time and timestamp support #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
adf5394
Add variable precision time and timestamp support for the experimenta…
lpoulain Aug 23, 2022
9d38bf9
Refactor _timestamp_map_func() and _time_map_func() for better readab…
lpoulain Oct 26, 2022
386e6ff
Split RowMapperFactory for time and timestamp processing
lpoulain Oct 27, 2022
d21007f
fixup! add type hints for the RowMapperFactory methods
lpoulain Oct 28, 2022
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
14 changes: 7 additions & 7 deletions tests/integration/test_dbapi_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ def test_datetime_query_param(trino_connection):
rows = cur.fetchall()

assert rows[0][0] == params
assert cur.description[0][1] == "timestamp"
assert cur.description[0][1] == "timestamp(6)"


def test_datetime_with_utc_time_zone_query_param(trino_connection):
Expand All @@ -295,7 +295,7 @@ def test_datetime_with_utc_time_zone_query_param(trino_connection):
rows = cur.fetchall()

assert rows[0][0] == params
assert cur.description[0][1] == "timestamp with time zone"
assert cur.description[0][1] == "timestamp(6) with time zone"


def test_datetime_with_numeric_offset_time_zone_query_param(trino_connection):
Expand All @@ -309,19 +309,19 @@ def test_datetime_with_numeric_offset_time_zone_query_param(trino_connection):
rows = cur.fetchall()

assert rows[0][0] == params
assert cur.description[0][1] == "timestamp with time zone"
assert cur.description[0][1] == "timestamp(6) with time zone"


def test_datetime_with_named_time_zone_query_param(trino_connection):
cur = trino_connection.cursor(experimental_python_types=True)

params = datetime(2020, 1, 1, 16, 43, 22, 320000, tzinfo=pytz.timezone('America/Los_Angeles'))
params = pytz.timezone('America/Los_Angeles').localize(datetime(2020, 1, 1, 16, 43, 22, 320000))

cur.execute("SELECT ?", params=(params,))
rows = cur.fetchall()

assert rows[0][0] == params
assert cur.description[0][1] == "timestamp with time zone"
assert cur.description[0][1] == "timestamp(6) with time zone"


def test_datetime_with_trailing_zeros(trino_connection):
Expand Down Expand Up @@ -371,7 +371,7 @@ def test_doubled_datetimes(trino_connection):
cur.execute("SELECT ?", params=(params,))
rows = cur.fetchall()

assert rows[0][0] == datetime(2002, 10, 27, 1, 30, 0, tzinfo=pytz.timezone('US/Eastern'))
assert rows[0][0] == pytz.timezone('US/Eastern').localize(datetime(2002, 10, 27, 1, 30, 0))

cur = trino_connection.cursor(experimental_python_types=True)

Expand All @@ -380,7 +380,7 @@ def test_doubled_datetimes(trino_connection):
cur.execute("SELECT ?", params=(params,))
rows = cur.fetchall()

assert rows[0][0] == datetime(2002, 10, 27, 1, 30, 0, tzinfo=pytz.timezone('US/Eastern'))
assert rows[0][0] == pytz.timezone('US/Eastern').localize(datetime(2002, 10, 27, 1, 30, 0))


def test_date_query_param(trino_connection):
Expand Down
155 changes: 155 additions & 0 deletions tests/integration/test_types_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import math
from datetime import timedelta, datetime, date, time
import pytest
import pytz
from decimal import Decimal
import trino

Expand Down Expand Up @@ -200,6 +202,159 @@ def test_digest(trino_connection):
.execute()


def test_date(trino_connection):
SqlTest(trino_connection) \
.add_field(sql="CAST(null AS DATE)", python=None) \
.add_field(sql="DATE '2001-08-22'", python=date(2001, 8, 22)) \
.add_field(sql="DATE '0001-01-01'", python=date(1, 1, 1)) \
Copy link
Copy Markdown
Member

@hashhar hashhar Nov 8, 2022

Choose a reason for hiding this comment

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

it's a good idea to have the dates sorted and to include a min value and max value so it's obvious what the boundaries are.

Also add a test to verify we get useful errors on querying years < 1 or years > 9999.

.add_field(sql="DATE '1582-10-04'", python=date(1582, 10, 4)) \
.add_field(sql="DATE '1582-10-05'", python=date(1582, 10, 5)) \
.add_field(sql="DATE '1582-10-14'", python=date(1582, 10, 14)) \
.execute()


def test_time(trino_connection):
time_0 = time(1, 23, 45)
time_3 = time(1, 23, 45, 123000)
time_6 = time(1, 23, 45, 123456)
time_round = time(1, 23, 45, 123457)
Copy link
Copy Markdown
Member

@hashhar hashhar Nov 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

we only test upwards rounding.

Add cases for:

  • TIME '00:00:00.0000004' -> TIME '00:00:00.000000' (minimal value with downwards rounding)
  • TIME '23:59:59.9999995' -> TIME '00:00:00.000000' (min value with upwards rounding)
  • TIME '23:59:59.9999999' -> TIME '00:00:00.000000' (max value with upwards rounding)
  • TIME '23:59:59.99999949999' -> TIME '23:59:59.999999' (round down)

Zero value with precision preserved

  • TIME '00:00:00' -> TIME '00:00:00'
  • TIME '00:00:00.000000' -> TIME '00:00:00.000000'

Copy link
Copy Markdown
Member

@hashhar hashhar Nov 8, 2022

Choose a reason for hiding this comment

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

Same for test_time_with_timezone

Copy link
Copy Markdown
Member Author

@lpoulain lpoulain Nov 9, 2022

Choose a reason for hiding this comment

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

Actually the rounding code does not work, and the only examples I could fine involve third party packages such as numpy. So open to suggestions for a better approach

Copy link
Copy Markdown
Member Author

@lpoulain lpoulain Nov 9, 2022

Choose a reason for hiding this comment

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

Is 23:59:59.999999949999 really supposed to be rounded down when converted to microseconds? Shouldn't 999999.949999 microseconds be rounded up?

Copy link
Copy Markdown
Member

@hashhar hashhar Nov 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry I added one more 9 in every case. Edited to correct.


SqlTest(trino_connection) \
.add_field(sql="CAST(null AS TIME)", python=None) \
.add_field(sql="CAST(null AS TIME(0))", python=None) \
.add_field(sql="CAST(null AS TIME(3))", python=None) \
.add_field(sql="CAST(null AS TIME(6))", python=None) \
.add_field(sql="CAST(null AS TIME(9))", python=None) \
.add_field(sql="CAST(null AS TIME(12))", python=None) \
.add_field(sql="CAST('01:23:45' AS TIME(0))", python=time_0) \
.add_field(sql="TIME '01:23:45.123'", python=time_3) \
.add_field(sql="CAST('01:23:45.123' AS TIME(3))", python=time_3) \
.add_field(sql="CAST('01:23:45.123456' AS TIME(6))", python=time_6) \
.add_field(sql="CAST('01:23:45.123456789' AS TIME(9))", python=time_round) \
.add_field(sql="CAST('01:23:45.123456789123' AS TIME(12))", python=time_round) \
.execute()


def test_time_with_timezone(trino_connection):
query_time_with_timezone(trino_connection, '-08:00')
query_time_with_timezone(trino_connection, '+08:00')
query_time_with_timezone(trino_connection, '+05:30')


def query_time_with_timezone(trino_connection, tz_str):
tz = datetime.strptime('+00:00', "%z").tzinfo

hours_shift = int(tz_str[:3])
minutes_shift = int(tz_str[4:])
delta = timedelta(hours=hours_shift, minutes=minutes_shift)

time_0 = (datetime(2, 1, 1, 11, 23, 45, 0) - delta).time().replace(tzinfo=tz)
time_3 = (datetime(2, 1, 1, 11, 23, 45, 123000) - delta).time().replace(tzinfo=tz)
time_6 = (datetime(2, 1, 1, 11, 23, 45, 123456) - delta).time().replace(tzinfo=tz)
time_round = (datetime(2, 1, 1, 11, 23, 45, 123457) - delta).time().replace(tzinfo=tz)

SqlTest(trino_connection) \
.add_field(sql="CAST(null AS TIME WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIME(0) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIME(3) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIME(6) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIME(9) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIME(12) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST('11:23:45 %s' AS TIME(0) WITH TIME ZONE)" % (tz_str), python=time_0) \
.add_field(sql="TIME '11:23:45.123 %s'" % (tz_str), python=time_3) \
.add_field(sql="CAST('11:23:45.123 %s' AS TIME(3) WITH TIME ZONE)" % (tz_str), python=time_3) \
.add_field(sql="CAST('11:23:45.123456 %s' AS TIME(6) WITH TIME ZONE)" % (tz_str), python=time_6) \
.add_field(sql="CAST('11:23:45.123456789 %s' AS TIME(9) WITH TIME ZONE)" % (tz_str), python=time_round) \
.add_field(sql="CAST('11:23:45.123456789123 %s' AS TIME(12) WITH TIME ZONE)" % (tz_str), python=time_round) \
.execute()


def test_timestamp(trino_connection):
timestamp_0 = datetime(2001, 8, 22, 1, 23, 45, 0)
timestamp_3 = datetime(2001, 8, 22, 1, 23, 45, 123000)
timestamp_6 = datetime(2001, 8, 22, 1, 23, 45, 123456)
timestamp_round = datetime(2001, 8, 22, 1, 23, 45, 123457)
Copy link
Copy Markdown
Member

@hashhar hashhar Nov 8, 2022

Choose a reason for hiding this comment

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

TIMESTAMP '1970-01-01 00:00:00.12345671' -> TIMESTAMP '1970-01-01 00:00:00.1234567' (round down)
TIMESTAMP '1970-01-01 00:00:00.1234567499' -> TIMESTAMP '1970-01-01 00:00:00.1234567' (round up trailing but overall value rounds down)
TIMESTAMP '1970-01-01 00:00:00.99999995' -> TIMESTAMP '1970-01-01 00:00:01.0000000' (round up to next second)
TIMESTAMP '1970-01-01 23:59:59.99999995' -> TIMESTAMP '1970-01-02 00:00:00.0000000' (round up to next day)
TIMESTAMP '1969-12-31 23:59:59.99999995' -> TIMESTAMP '1970-01-01 00:00:00.0000000' (round up from before epoch to epoch)

Copy link
Copy Markdown
Member

@hashhar hashhar Nov 8, 2022

Choose a reason for hiding this comment

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

Also sort according to value + add min and max supported value + verify what error we get for max.

timestamp_ce = datetime(1, 1, 1, 1, 23, 45, 123000)
timestamp_julian = datetime(1582, 10, 4, 1, 23, 45, 123000)
timestamp_during_switch = datetime(1582, 10, 5, 1, 23, 45, 123000)
timestamp_gregorian = datetime(1582, 10, 14, 1, 23, 45, 123000)

SqlTest(trino_connection) \
.add_field(sql="CAST(null AS TIMESTAMP)", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(0))", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(3))", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(6))", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(9))", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(12))", python=None) \
.add_field(sql="CAST('2001-08-22 01:23:45' AS TIMESTAMP(0))", python=timestamp_0) \
.add_field(sql="TIMESTAMP '2001-08-22 01:23:45.123'", python=timestamp_3) \
.add_field(sql="TIMESTAMP '0001-01-01 01:23:45.123'", python=timestamp_ce) \
.add_field(sql="TIMESTAMP '1582-10-04 01:23:45.123'", python=timestamp_julian) \
.add_field(sql="TIMESTAMP '1582-10-05 01:23:45.123'", python=timestamp_during_switch) \
.add_field(sql="TIMESTAMP '1582-10-14 01:23:45.123'", python=timestamp_gregorian) \
.add_field(sql="CAST('2001-08-22 01:23:45.123' AS TIMESTAMP(3))", python=timestamp_3) \
.add_field(sql="CAST('2001-08-22 01:23:45.123456' AS TIMESTAMP(6))", python=timestamp_6) \
.add_field(sql="CAST('2001-08-22 01:23:45.123456111' AS TIMESTAMP(9))", python=timestamp_6) \
.add_field(sql="CAST('2001-08-22 01:23:45.123456789' AS TIMESTAMP(9))", python=timestamp_round) \
.add_field(sql="CAST('2001-08-22 01:23:45.123456111111' AS TIMESTAMP(12))", python=timestamp_6) \
.add_field(sql="CAST('2001-08-22 01:23:45.123456789123' AS TIMESTAMP(12))", python=timestamp_round) \
.execute()


def test_timestamp_with_timezone(trino_connection):
Copy link
Copy Markdown
Member

@hashhar hashhar Sep 14, 2022

Choose a reason for hiding this comment

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

Squash "Refactor some test function names for consistency" since it changes something introduced in the first commit itself.

query_timestamp_with_timezone(trino_connection, '-08:00')
query_timestamp_with_timezone(trino_connection, '+08:00')
query_timestamp_with_timezone(trino_connection, '+05:30')
query_timestamp_with_timezone(trino_connection, 'US/Eastern')
query_timestamp_with_timezone(trino_connection, 'Asia/Kolkata')
query_timestamp_with_timezone(trino_connection, 'GMT')


def query_timestamp_with_timezone(trino_connection, tz_str):
if tz_str.startswith('+') or tz_str.startswith('-'):
hours_shift = int(tz_str[:3])
minutes_shift = int(tz_str[4:])
else:
tz = pytz.timezone(tz_str)
offset = tz.utcoffset(datetime.now())
offset_seconds = offset.total_seconds()
hours_shift = int(offset_seconds / 3600)
minutes_shift = offset_seconds % 3600 / 60

tz = pytz.timezone('Etc/GMT')
delta = timedelta(hours=hours_shift, minutes=minutes_shift)

timestamp_0 = tz.localize(datetime(2001, 8, 22, 11, 23, 45, 0)) - delta
timestamp_3 = tz.localize(datetime(2001, 8, 22, 11, 23, 45, 123000)) - delta
timestamp_6 = tz.localize(datetime(2001, 8, 22, 11, 23, 45, 123456)) - delta
timestamp_round = tz.localize(datetime(2001, 8, 22, 11, 23, 45, 123457)) - delta
Comment on lines +327 to +330
Copy link
Copy Markdown
Member

@hashhar hashhar Nov 8, 2022

Choose a reason for hiding this comment

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

this is not how timezones work. Depending on exact date the shift value can be different.

i.e. offset = tz.utcoffset(datetime.now()) will give different offsets based on different time of year or dates.

Is there no way to "apply" a timezone to a datetime?


SqlTest(trino_connection) \
.add_field(sql="CAST(null AS TIMESTAMP WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(0) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(3) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(6) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(9) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST(null AS TIMESTAMP(12) WITH TIME ZONE)", python=None) \
.add_field(sql="CAST('2001-08-22 11:23:45 %s' AS TIMESTAMP(0) WITH TIME ZONE)" % (tz_str),
python=timestamp_0) \
.add_field(sql="TIMESTAMP '2001-08-22 11:23:45.123 %s'" % (tz_str),
python=timestamp_3) \
.add_field(sql="CAST('2001-08-22 11:23:45.123 %s' AS TIMESTAMP(3) WITH TIME ZONE)" % (tz_str),
python=timestamp_3) \
.add_field(sql="CAST('2001-08-22 11:23:45.123456 %s' AS TIMESTAMP(6) WITH TIME ZONE)" % (tz_str),
python=timestamp_6) \
.add_field(sql="CAST('2001-08-22 11:23:45.123456111 %s' AS TIMESTAMP(9) WITH TIME ZONE)" % (tz_str),
python=timestamp_6) \
.add_field(sql="CAST('2001-08-22 11:23:45.123456789 %s' AS TIMESTAMP(9) WITH TIME ZONE)" % (tz_str),
python=timestamp_round) \
.add_field(sql="CAST('2001-08-22 11:23:45.123456111111 %s' AS TIMESTAMP(12) WITH TIME ZONE)" % (tz_str),
python=timestamp_6) \
.add_field(sql="CAST('2001-08-22 11:23:45.123456789123 %s' AS TIMESTAMP(12) WITH TIME ZONE)" % (tz_str),
python=timestamp_round) \
.execute()


class SqlTest:
def __init__(self, trino_connection):
self.cur = trino_connection.cursor(experimental_python_types=True)
Expand Down
Loading