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

Jump to bottom

Extend data types support for UTC dates, i.e. Timestamp Athena type #49

Open
ssedano opened this issue Nov 21, 2020 · 7 comments · May be fixed by #67
Open

Extend data types support for UTC dates, i.e. Timestamp Athena type #49

ssedano opened this issue Nov 21, 2020 · 7 comments · May be fixed by #67

Comments

@ssedano
Copy link

ssedano commented Nov 21, 2020

Hi team,

I'd like to extend data types support for UTC dates, i.e. Timestamp Athena type. My proposal is to create a new option, e.g. useUtcDates, to cast values of columns with type Timestamp to Date Javascript objects. This setting would have a default value of false. The caveat of this setting is that it will use machine-local time, thus this setting is intended for systems configured in UTC. This could be extended to Date Type but it would transform dates into date and times, e.g. '2020-11-18' would become '2020-11-18T00:00:00Z'. I intentionally avoid casting timestamp with time zone Athena type to reduce complexity.

For applications that deal with dates, converting Timestamp columns to Date objects is a common task given that many libraries expect Date objects. Moreover, athena-express hides the type in the result set. This forces developers that wish to operate using dates to access the column by name and explicitly convert values to Date objects.

I'd be willing to submit a patch if this feature request is approved.

Proposed update to documentation:

  • useUtcDates | boolean | (default false) | Cast Timestamp types to Javascript Date objects with times in UTC. Set this option if your code runs in machines configured with UTC as their timezone. Useful when your Timestamp columns are representing UTC date and time and you want to work with Date objects instead of String objects
@ghdna
Copy link
Owner

ghdna commented Apr 16, 2021

I somehow missed responding to this. If you wanna go ahead and add that, I'll be happy to take a look. Just consider any edge cases that might break it.

@ghdna
Copy link
Owner

ghdna commented Apr 25, 2021

Actually the more I think about this, the more it can cause more confusion for users since there are several date formats to convert into:
For e.g. Athena's TIMESTAMP datatype will fetch results as 2008-09-15 03:04:05.327
This string can be converted into 5 formats:

  1. new Date() => Mon Sep 15 2008 03:04:05 GMT-0400 (Eastern Daylight Time)
  2. .toUTCString() => "Mon, 15 Sep 2008 07:04:05 GMT"
  3. .toDateString() => "Mon Sep 15 2008"
  4. .toISOString() => "2008-09-15T07:04:05.327Z"
  5. .toTimeString() => "03:04:05 GMT-0400 (Eastern Daylight Time)"

How would AthenaExpress know which format the user cares about? And the bigger question is, should this be something AthenaExpress does or should this functionality be handed over to the user to decide.

@ssedano
Copy link
Author

ssedano commented Apr 27, 2021 •
edited
Loading

Thank you for your reply.

should this be something AthenaExpress does or should this functionality be handed over to the user to decide.

Is the right question to ask. Unfortunately, query method doesn't include the data type. Hence why I propose to let the user decide at the time of configuring AthenaExpress. This saves the user from tracking column types separately and casting objects, keeping AthenaExpress' simplicity.

@ghdna
Copy link
Owner

ghdna commented Apr 29, 2021

ok i understand. Let me check on the effort required for this

@ssedano
Copy link
Author

ssedano commented May 3, 2021

I would be happy to contribute the patch

Thank you.

@ghdna
Copy link
Owner

ghdna commented May 5, 2021

Sure, please do. I'd be happy to look at it.

@ssedano
Copy link
Author

ssedano commented Jun 6, 2021

Apologies for the delay on this. I expect to have a pull request ready in the next two weeks

@ssedano ssedano linked a pull request Aug 2, 2021 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Extend type support for dates and timestamps ssedano/athena-express
2 participants
@ssedano @ghdna