Skip to content

Use time instead of timetz when adding Time columns #4298

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

Closed
seancolsen opened this issue Mar 3, 2025 · 2 comments · Fixed by #4346
Closed

Use time instead of timetz when adding Time columns #4298

seancolsen opened this issue Mar 3, 2025 · 2 comments · Fixed by #4346
Labels
good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. help wanted Community contributors can implement this ready Ready for implementation type: enhancement work: frontend Related to frontend code in the mathesar_ui directory
Milestone

Comments

@seancolsen
Copy link
Contributor

seancolsen commented Mar 3, 2025

Context

PostgreSQL strongly recommends against using the timetz type (aka time with time zone).

  • From https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_timetz

    Don't use the timetz type. You probably want timestamptz instead.

    Why not?

    Even the manual tells you it's only implemented for SQL compliance.

    The type time with time zone is defined by the SQL standard, but the definition exhibits properties which lead to questionable usefulness. In most cases, a combination of date, time, timestamp without time zone, and timestamp with time zone should provide a complete range of date/time functionality required by any application.

    When should you?

    Never.

  • From https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-TIMEZONES

    We do not recommend using the type time with time zone (though it is supported by PostgreSQL for legacy applications and for compliance with the SQL standard).

  • From https://www.postgresql.org/docs/17/functions-datetime.html#FUNCTIONS-DATETIME-ZONECONVERT

    Assuming the current TimeZone setting is America/Los_Angeles:

    SELECT TIME WITH TIME ZONE '20:38:40-05' AT LOCAL;

    Result: 17:38:40

    [This] example is a cautionary tale. Due to the fact that there is no date associated with the input value, the conversion is made using the current date of the session. Therefore, this static example may show a wrong result depending on the time of the year it is viewed because 'America/Los_Angeles' observes Daylight Savings Time.

In Mathesar

As far as I can tell, there are three different ways the user can end up creating a Time column from Mathesar:

  1. Casting a text column to Time.

  2. Importing a CSV containing time data.

  3. Adding: a new column to a table and selecting the Time type during column creation.

When casting and importing, Mathesar defaults to time without time zone. Good.

But when adding a column to an existing table, Mathesar defaults to time with time zone.

I think we should change this default for newly-added columns to time without time zone.

This is a frontend issue because it's the front end that sends chooses the DB type when adding a new column.

@seancolsen seancolsen added needs: product approval It's not yet clear that this issue will actually improve Mathesar from a user's perspective ready Ready for implementation type: enhancement work: frontend Related to frontend code in the mathesar_ui directory labels Mar 3, 2025
@seancolsen seancolsen added this to the Backlog milestone Mar 3, 2025
@seancolsen
Copy link
Contributor Author

@mathemancer can you weigh in on whether you think this is a good idea. If you sign off on it, then I think we could remove needs: product approval and label this good first issue.

@seancolsen seancolsen removed the ready Ready for implementation label Mar 3, 2025
@mathemancer
Copy link
Contributor

It's a good idea, and I imagine this would in fact be an easy first issue for someone.

@mathemancer mathemancer added good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. help wanted Community contributors can implement this ready Ready for implementation and removed needs: product approval It's not yet clear that this issue will actually improve Mathesar from a user's perspective labels Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. help wanted Community contributors can implement this ready Ready for implementation type: enhancement work: frontend Related to frontend code in the mathesar_ui directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants