Skip to content
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

Improve date/time hint handling for Postgres based on better testing #146

Merged
merged 4 commits into from
Dec 9, 2020
Merged

Improve date/time hint handling for Postgres based on better testing #146

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
96464f1
Improve date/time hint handling for Postgres based on better testing
vinceatbluelabs Dec 9, 2020
19c044f
Request bigfiles slip
vinceatbluelabs Dec 9, 2020
dc67d92
Ratchet coverage
vinceatbluelabs Dec 9, 2020
e4970e2
Ratchet
vinceatbluelabs Dec 9, 2020
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
2 changes: 1 addition & 1 deletion metrics/bigfiles_high_water_mark
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1094
1143
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See note below about the growth in date_input_style.py and what we can do in the future now that we have better tests...

389: tests/integration/*****
381: records_mover/db/postgres/copy_options/date_input_style.py
373: setup.py

Reduce total number of bigfiles violations to 1094 or below!

Tasks: TOP => quality
(See full trace by running task with --trace)
Existing violations: 1094
Found 1143 bigfiles violations

153 changes: 152 additions & 1 deletion records_mover/db/postgres/copy_options/date_input_style.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def upgrade_date_order_style(style: DateOrderStyle, hint_name: str) -> None:
# interpretation, DMY to select day-month-year interpretation, or
# YMD to select year-month-day interpretation."

# $ cd tests/integration
# $ ./itest shell
# $ db dockerized-postgres
# postgres=# SHOW DateStyle;
Expand All @@ -50,6 +51,12 @@ def upgrade_date_order_style(style: DateOrderStyle, hint_name: str) -> None:

# Default value is "YYYY-MM-DD HH:MI:SSOF".

#
# To get a postgres database to test these cases with:
#
# cd tests/integration
# ./itest shell
# db dockerized-postgres
if datetimeformattz in ['YYYY-MM-DD HH:MI:SSOF',
'YYYY-MM-DD HH24:MI:SSOF']:
#
Expand Down Expand Up @@ -87,6 +94,46 @@ def upgrade_date_order_style(style: DateOrderStyle, hint_name: str) -> None:
#
# postgres=#
upgrade_date_order_style('MDY', 'datetimeformattz')
elif datetimeformattz == 'DD-MM-YY HH:MI:SSOF':
# postgres=# select timestamptz '02-01-2999 23:01:01-10';
# timestamptz
# ------------------------
# 2999-02-02 09:01:01+00
# (1 row)
#
upgrade_date_order_style('DMY', 'datetimeformattz')
elif datetimeformattz == 'DD/MM/YY HH:MI:SSOF':
# postgres=# select timestamptz '02/01/99 23:01:01-10';
# timestamptz
# ------------------------
# 1999-02-02 09:01:01+00
# (1 row)
#
upgrade_date_order_style('DMY', 'datetimeformattz')
elif datetimeformattz == 'DD-MM-YYYY HH:MI:SSOF':
# postgres=# select timestamptz '02-01-1999 23:01:01-10';
# timestamptz
# ------------------------
# 1999-02-02 09:01:01+00
# (1 row)
#
upgrade_date_order_style('DMY', 'datetimeformattz')
elif datetimeformattz == 'MM/DD/YY HH:MI:SSOF':
# postgres=# select timestamptz '01/02/99 23:01:01-10';
# timestamptz
# ------------------------
# 1999-01-03 09:01:01+00
# (1 row)
#
upgrade_date_order_style('MDY', 'datetimeformattz')
elif datetimeformattz == 'MM-DD-YYYY HH:MI:SSOF':
# postgres=# select timestamptz '01-02-1999 23:01:01-10';
# timestamptz
# ------------------------
# 1999-01-03 09:01:01+00
# (1 row)
#
upgrade_date_order_style('MDY', 'datetimeformattz')
else:
cant_handle_hint(fail_if_cant_handle_hint, 'datetimeformattz', hints)

Expand Down Expand Up @@ -131,6 +178,69 @@ def upgrade_date_order_style(style: DateOrderStyle, hint_name: str) -> None:
#
# postgres=#

upgrade_date_order_style('MDY', 'datetimeformat')
elif datetimeformat == 'DD-MM-YY HH12:MI AM':
# postgres=# select timestamp '02-01-20 3:23 PM';
# timestamp
# ---------------------
# 2020-02-01 15:23:00
# (1 row)
#
# postgres=#
upgrade_date_order_style('DMY', 'datetimeformat')
elif datetimeformat == 'DD/MM/YY HH12:MI AM':
# postgres=# select timestamp '02/01/20 3:23 PM';
# timestamp
# ---------------------
# 2020-02-01 15:23:00
# (1 row)
#
# postgres=#
upgrade_date_order_style('DMY', 'datetimeformat')
elif datetimeformat == 'DD-MM-YYYY HH12:MI AM':
# postgres=# select timestamp '02-01-2020 3:23 PM';
# timestamp
# ---------------------
# 2020-02-01 15:23:00
# (1 row)
#
# postgres=#
upgrade_date_order_style('DMY', 'datetimeformat')
elif datetimeformat == 'MM-DD-YY HH12:MI AM':
# postgres=# select timestamp '02-01-20 3:23 PM';
# timestamp
# ---------------------
# 2020-02-01 15:23:00
# (1 row)
#
# postgres=#
upgrade_date_order_style('MDY', 'datetimeformat')
elif datetimeformat == 'MM/DD/YY HH12:MI AM':
# postgres=# select timestamp '02/01/20 3:23 PM';
# timestamp
# ---------------------
# 2020-02-01 15:23:00
# (1 row)
#
# postgres=#
upgrade_date_order_style('MDY', 'datetimeformat')
elif datetimeformat == 'MM-DD-YYYY HH12:MI AM':
# postgres=# select timestamp '01-02-2020 3:23 PM';
# timestamp
# ---------------------
# 2020-01-02 15:23:00
# (1 row)
#
# postgres=#
upgrade_date_order_style('MDY', 'datetimeformat')
elif datetimeformat == 'MM-DD-YYYY HH:MI:SS':
# postgres=# select timestamp '01-02-2020 3:23 PM';
# timestamp
# ---------------------
# 2020-01-02 15:23:00
# (1 row)
#
# postgres=#
upgrade_date_order_style('MDY', 'datetimeformat')
else:
cant_handle_hint(fail_if_cant_handle_hint, 'datetimeformat', hints)
Expand Down Expand Up @@ -165,10 +275,33 @@ def upgrade_date_order_style(style: DateOrderStyle, hint_name: str) -> None:
#
# postgres=#

# Supported!
quiet_remove(unhandled_hints, 'timeonlyformat')
elif timeonlyformat == "HH:MI:SS":
# postgres=# select time '13:00:00';
# time
# ----------
# 13:00:00
# (1 row)
#
# postgres=#
quiet_remove(unhandled_hints, 'timeonlyformat')
elif timeonlyformat == "HH24:MI":

# "HH24:MI" (e.g., "13:00")
#
# postgres=# select time '13:00';
# time
# ----------
# 13:00:00
# (1 row)
#
# postgres=#

# Supported!
quiet_remove(unhandled_hints, 'timeonlyformat')
else:
cant_handle_hint(fail_if_cant_handle_hint, 'datetimeformat', hints)
cant_handle_hint(fail_if_cant_handle_hint, 'timeonlyformat', hints)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops! This resulted in printing the wrong diagnostic error message.


# dateformat: Valid values: null, "YYYY-MM-DD", "MM-DD-YYYY", "DD-MM-YYYY", "MM/DD/YY".
dateformat = hints.dateformat
Expand Down Expand Up @@ -206,6 +339,15 @@ def upgrade_date_order_style(style: DateOrderStyle, hint_name: str) -> None:
#
# postgres=#
upgrade_date_order_style('DMY', 'dateformat')
elif dateformat == "DD-MM-YY":
# postgres=# select date '02-01-99';
# date
# ------------
# 1999-02-01
# (1 row)
#
# postgres=#
upgrade_date_order_style('DMY', 'dateformat')
elif dateformat == "MM/DD/YY":
# "MM/DD/YY".
#
Expand All @@ -217,6 +359,15 @@ def upgrade_date_order_style(style: DateOrderStyle, hint_name: str) -> None:
#
# postgres=#
upgrade_date_order_style('MDY', 'dateformat')
elif dateformat == "DD/MM/YY":
# postgres=# select date '02/01/99';
# date
# ------------
# 1999-02-01
# (1 row)
#
# postgres=#
upgrade_date_order_style('DMY', 'dateformat')
elif dateformat is None:
# null implies that that date format is unknown, and that the
# implementation SHOULD generate using their default value and
Copy link
Contributor Author

@vinceatbluelabs vinceatbluelabs Dec 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

In general you can see this file is very case-based. I think it'd both be less exhausting as a piece of code and more helpful for folks if it were turned into a more dynamic function that didn't need to check every single supported case, but instead did something more dynamic (e.g., "is DD before MM in the string in front of me?). That'd also be one less piece to improve before we can advertise general support for date/time format strings in our hints and loosen the Records Spec accordingly.

Expand Down
5 changes: 3 additions & 2 deletions records_mover/db/postgres/copy_options/date_output_style.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ def determine_date_output_style(unhandled_hints: Set[str],
date_order_style: Optional[DateOrderStyle] = None

if (dateformat == 'YYYY-MM-DD' and
timeonlyformat == 'HH24:MI:SS' and
timeonlyformat in ['HH24:MI:SS', 'HH:MI:SS'] and
datetimeformattz in ['YYYY-MM-DD HH:MI:SSOF',
'YYYY-MM-DD HH24:MI:SSOF'] and
datetimeformat == 'YYYY-MM-DD HH24:MI:SS'):
datetimeformat in ['YYYY-MM-DD HH24:MI:SS',
'YYYY-MM-DD HH:MI:SS']):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

HH is equivalent to HH24 in our spec.

date_output_style: DateOutputStyle = 'ISO'
# date_order_style doesn't really matter, as ISO is not ambiguous
else:
Expand Down
Loading