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.

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

Fix date literals #532

Closed
wants to merge 6 commits into from
Closed

Conversation

kozak
Copy link

@kozak kozak commented Nov 3, 2021

Reason why this is needed: Postgresql doesn't know how to parse:
25-10-20T22:36:00Z because both d-m-y and y-m-d formats could work.
Will add explicit padding to year to make it clear.

The previous format %F is %Y-%m-%d where %Y is unpadded.

The behaviour is illustrated below:

=> select '02-10-20T22:36:00Z' :: timestamp;
  timestamp
  ---------------------
  2020-02-10 22:36:00



=> select '0020-10-20T22:36:00Z' :: timestamp;
  timestamp
  ---------------------
  0020-10-20 22:36:00



=> select '020-10-20T22:36:00Z' :: timestamp;
  timestamp
  ---------------------
  0020-10-20 22:36:00



=> select '20-10-20T22:36:00Z' :: timestamp;
  ERROR:  22008: date/time field value out of range: "20-10-20T22:36:00Z"
  LINE 1: select '20-10-20T22:36:00Z' :: timestamp; ^
  HINT:  Perhaps you need a different "datestyle" setting.
  LOCATION:  DateTimeParseError, datetime.c:3763

=> select '12-10-20T22:36:00Z' :: timestamp;
  timestamp
  ---------------------
  2020-12-10 22:36:00

=> select '25-10-20T22:36:00Z' :: timestamp;
  ERROR:  22008: date/time field value out of range: "25-10-20T22:36:00Z" 
  LINE 1: select '25-10-20T22:36:00Z' :: timestamp;               ^
  HINT:  Perhaps you need a different "datestyle" setting.
  LOCATION:  DateTimeParseError, datetime.c:3763

=> select '020-10-20T22:36:00Z' :: timestamp;
  timestamp
  ---------------------
    0020-10-20 22:36:00
    (1 row)

For SQLite, the datetime function produces NULL for the non-padded versions.

@tomjaguarpaw
Copy link
Owner

Hello, thanks for your interest. I'm not sure what behaviour you are trying to change exactly. Could you provide an example of Haskell code that does not work before but that does work after?

@kozak
Copy link
Author

kozak commented Nov 4, 2021

Hello :) Please have a look at the "padded" family of tests. Those tests fail without the change to date formatting with the same error as the one below:

=> select '20-10-20T22:36:00Z' :: timestamp;
  ERROR:  22008: date/time field value out of range: "20-10-20T22:36:00Z"
  LINE 1: select '20-10-20T22:36:00Z' :: timestamp; ^
  HINT:  Perhaps you need a different "datestyle" setting.
  LOCATION:  DateTimeParseError, datetime.c:3763

It seems PostregreSQL doesn't seem to know how to parse "20-10-20T22:36:00Z", because both "YY-MM-DD" and "DD-MM-YY" could be valid.

@tomjaguarpaw
Copy link
Owner

Ah, I see.

> formatTime defaultTimeLocale "%F" (fromGregorian 1 2 3)
"1-02-03"
> formatTime defaultTimeLocale "%0Y-%m-%d" (fromGregorian 1 2 3)
"0001-02-03"
> iso8601Show (fromGregorian 1 2 3)
"0001-02-03"

For you to do please @kozak:

  • Change your PR to use iso8601Show
  • Please minimise the diff by removing all the white space and import-reordering changes in your PR

@tomjaguarpaw
Copy link
Owner

I removed the usage of read, which was muddying the waters so your branch will need rebasing on latest master. However, I can do that for you if you like.

tomjaguarpaw added a commit that referenced this pull request Nov 4, 2021
@tomjaguarpaw
Copy link
Owner

Thanks very much for spotting and fixing this! Pushed to master as 2995835.

(That commit is Postgres only. Do you really want this for SQLite too?)

@tomjaguarpaw
Copy link
Owner

Closing as fixed separately. @kozak please comment on #533 if you really want this in opaleye-sqlite.

@kozak
Copy link
Author

kozak commented Nov 4, 2021

Thanks @tomjaguarpaw :) Nope we can ignore SQLite.

@tomjaguarpaw
Copy link
Owner

Cool, thanks for your help on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants