mathesar-424 Handle `TIMESTAMP` data type in the backend #865

silentninja · 2021-12-03T22:58:13Z

Fixes #424 Handle Timestamp data type in the backend.

This PR introduces Timestamp data type(with and without timezone) and relevant functions to convert text data types that contain a date-time string into a timestamp data type.

The order by which the data type is inferred will change to the following

If a text contains a date string without any time information date type will be inferred.
If a text contains a date string along with time information but does not include a timezone information timestamp without timezone data type will be inferred.
If a text contains a date string along with time and timezone information, timestamp with timezone data type will be introduced. This type will be inferred if only one or more rows contains the necessary information as other data type will end up with losing the information

Technical details
Adds TImestamp with timezone and Timestamp without timezone data type and Relevant casting function that restricts the data that can be converted to the data type.
The casting function of date data type has been limited to accept only text with date element that does not contain time information
Screenshots

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the master branch of the repository
My commit messages follow best practices.
My code follows the established code style of the repository.
I added tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no
visible errors.

TaskList

Create a timestamp column(db/tests/columns/operations/test_create.py)
Automatic type inference during file import suggests (with time zone or without timezone) when it makes sense to do so (db/tests/tables/operations/test_infer_types.py:31)
Users can use the API to change a column to TIMESTAMP(with time zone or without timezone) if it's possible to do so (db/tests/types/operations/test_cast.py:169)
Users can set and change the precision of a given TIMESTAMP column via a type_options field in the API(db/tests/types/operations/test_cast.py:825)
Fetching a timestamp record(mathesar/tests/api/test_record_api.py)

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Add support for timestamp with timezone and timestamp without timezone data type Add casting function for `timestamp without timezone` to cast from text containing a date time string without timezone to `timestamp without timezone` data type Update the date type casting function to not accept data that contains additional time information as it will be handled by timestamp data type Change the spelling from `TIMESTAMP_WITH_TIMESTAMP_ZONE` to `TIMESTAMP_WITH_TIME_ZONE` in `db/types/base.py`

…turns the same null value when casting it to different type, messing up with the type checking logic. Revert test cases that were accidentally deleted with previous commit

…amp with timezone and timestamp without timezone

dmos62 · 2021-12-07T17:06:27Z

db/tests/types/operations/test_cast.py

+        ISCHEMA_NAME: PostgresType.TIMESTAMP_WITHOUT_TIME_ZONE.value,
+        REFLECTED_NAME: TIMESTAMP_WITHOUT_TIME_ZONE,
+        TARGET_DICT: {
+            CHAR: {VALID: []},


I presume a timestamp should be castable to any text/character type, including CHAR.

It is possible to cast a timestamp to CHAR, but CHAR returns exact number of characters specified and this causes problems with testing as the type_options that specifies length has to be specified with each test case, which is not possible right now. I have created a issue that should help with testing such cases

kgodey

I'll leave this to @mathemancer to review/merge. I took a quick glance and don't see any issues.

kgodey · 2021-12-08T20:21:25Z

Updated status to blocked since it's blocked by #628

# Conflicts: # db/types/datetime.py

silentninja · 2021-12-21T08:54:16Z

@silentninja Have you noticed the requested changes on this PR?

Yes, I did notice it, sorry for not changing the status. I got caught up with other issues, will be working on it today.

…ith just date.

… that midnight timestamp string won't affect the cast Add date to timestamp casting function Add timestamp without timezone to timestamp with timezone casting functiong

silentninja · 2021-12-23T05:42:34Z

see the PostgreSQL docs about RETURNS NULL ON NULL INPUT: https://www.postgresql.org/docs/13/sql-createfunction.html

I didn't want to modify the db.types.operations.cast.assemble_function_creation_sql as it is being used by other casting functions.

Moreover, I am not sure if this special case where performance drops applies in our situation as it mostly applies to table functions but wanted to give it the benefit of doubt

…tion to remove unnecessary null checks

…ally

mathemancer

I think this should work, and the choices w.r.t. what casts to what else seem logical.

About setting the functions to RETURNS NULL ON NULL INPUT, that should be safe since these are all casting functions that definitely ought to return null on null input. Also, many of them can't be inlined for various reasons. I'll make a separate issue to explore that, though, since some of them can be inlined, and these casting functions get run for every row of some table or another quite often. For now, it's a moot point, since the null checks all got removed (unless we do see some improvement overall in performance).

silentninja · 2022-01-06T16:15:06Z

@mathemancer Thanks! I will change the strict keyword to RETURNS NULL ON NULL INPUT as it seems be descriptive compared to STRICT

silentninja · 2022-01-06T16:40:06Z

@kgodey We need to specify that the datetime casting functions are not transaction-safe and can affect timezones when run in the same transaction, where should this be documented?

…ULL INPUT` as it is descriptive

kgodey · 2022-01-06T16:57:04Z

@silentninja Please document it in the code and in the wiki (write a short spec that's linked to from "Technical Specs" in https://wiki.mathesar.org/en/engineering/architecture). Please also explain what "can affect timezones" means in greater detail.

kgodey · 2022-01-06T17:04:40Z

@silentninja Feel free to merge this PR when you're ready.

…n to cast a data type to timestamp without timezone

silentninja requested review from a team, dmos62 and pavish and removed request for a team December 3, 2021 22:58

silentninja marked this pull request as draft December 3, 2021 22:58

github-actions bot requested review from eito-fis, kgodey, mathemancer, powellc and seancolsen December 3, 2021 22:58

kgodey assigned kgodey and mathemancer Dec 4, 2021

kgodey added the pr-status: review A PR awaiting review label Dec 4, 2021

silentninja linked an issue Dec 4, 2021 that may be closed by this pull request

Handle TIMESTAMP data type in the backend #424

Closed

silentninja added 2 commits December 7, 2021 03:13

Exclude comparing null values when casting to datetime types as it re…

3bd0b7c

…turns the same null value when casting it to different type, messing up with the type checking logic. Revert test cases that were accidentally deleted with previous commit

Remove timestamp data type from types_map as it is replaced by timest…

ec34603

…amp with timezone and timestamp without timezone

dmos62 reviewed Dec 7, 2021

View reviewed changes

kgodey reviewed Dec 8, 2021

View reviewed changes

kgodey removed their assignment Dec 8, 2021

kgodey added needs: unblocking Blocked by other work and removed pr-status: review A PR awaiting review labels Dec 8, 2021

Base automatically changed from time_type to master December 13, 2021 08:22

silentninja added status: started and removed needs: unblocking Blocked by other work labels Dec 13, 2021

silentninja added 2 commits December 13, 2021 17:24

Merge branch 'master' into timestamp-type

1a2cdf5

# Conflicts: # db/types/datetime.py

Fix lint errors

8ed57e2

silentninja marked this pull request as ready for review December 13, 2021 14:11

github-actions bot requested a review from dmos62 December 13, 2021 14:12

silentninja assigned silentninja and unassigned mathemancer Dec 21, 2021

silentninja added 3 commits December 21, 2021 14:30

Merge branch 'master' into timestamp-type

a49bc7f

Allow timestamp with timezone and without timezone to accept string w…

a00c6a3

…ith just date.

Remove date comparison when casting to timestamp without timezone, so…

421bd92

… that midnight timestamp string won't affect the cast Add date to timestamp casting function Add timestamp without timezone to timestamp with timezone casting functiong

silentninja added 2 commits December 23, 2021 14:23

Convert function used to cast to different data type as a strict func…

33c0843

…tion to remove unnecessary null checks

Fix datetime casting function to set timezone locally instead of glob…

a32238c

…ally

silentninja requested a review from mathemancer December 23, 2021 11:27

Merge branch 'master' into timestamp-type

349981b

kgodey assigned mathemancer and unassigned silentninja Dec 23, 2021

kgodey added pr-status: review A PR awaiting review and removed pr-status: revision A PR awaiting follow-up work from its author after review labels Dec 23, 2021

Merge branch 'master' into timestamp-type

93df286

mathemancer approved these changes Jan 6, 2022

View reviewed changes

silentninja added pr-status: revision A PR awaiting follow-up work from its author after review and removed pr-status: review A PR awaiting review labels Jan 6, 2022

silentninja assigned silentninja and unassigned mathemancer Jan 6, 2022

Change casting function parameter from Strict to `RETURNS NULL ON N…

75c88d3

…ULL INPUT` as it is descriptive

silentninja added 2 commits January 7, 2022 03:21

Add comments to warn about the timezone leak when calling the functio…

149a86b

…n to cast a data type to timestamp without timezone

Merge branch 'master' into timestamp-type

03bc152

silentninja merged commit 5908c50 into master Jan 6, 2022

silentninja deleted the timestamp-type branch January 6, 2022 22:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mathesar-424 Handle `TIMESTAMP` data type in the backend #865

mathesar-424 Handle `TIMESTAMP` data type in the backend #865

silentninja commented Dec 3, 2021 •

edited

Loading

dmos62 Dec 7, 2021

silentninja Dec 7, 2021 •

edited

Loading

kgodey left a comment •

edited

Loading

kgodey commented Dec 8, 2021

silentninja commented Dec 21, 2021

silentninja commented Dec 23, 2021

mathemancer left a comment

silentninja commented Jan 6, 2022

silentninja commented Jan 6, 2022

kgodey commented Jan 6, 2022

kgodey commented Jan 6, 2022

mathesar-424 Handle TIMESTAMP data type in the backend #865

mathesar-424 Handle TIMESTAMP data type in the backend #865

Conversation

silentninja commented Dec 3, 2021 • edited Loading

Checklist

TaskList

Developer Certificate of Origin

dmos62 Dec 7, 2021

Choose a reason for hiding this comment

silentninja Dec 7, 2021 • edited Loading

Choose a reason for hiding this comment

kgodey left a comment • edited Loading

Choose a reason for hiding this comment

kgodey commented Dec 8, 2021

silentninja commented Dec 21, 2021

silentninja commented Dec 23, 2021

mathemancer left a comment

Choose a reason for hiding this comment

silentninja commented Jan 6, 2022

silentninja commented Jan 6, 2022

kgodey commented Jan 6, 2022

kgodey commented Jan 6, 2022

mathesar-424 Handle `TIMESTAMP` data type in the backend #865

mathesar-424 Handle `TIMESTAMP` data type in the backend #865

silentninja commented Dec 3, 2021 •

edited

Loading

silentninja Dec 7, 2021 •

edited

Loading

kgodey left a comment •

edited

Loading