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

Handle INTERVAL type in the backend #430

Closed
Tracked by #251
kgodey opened this issue Jul 18, 2021 · 5 comments · Fixed by #1067
Closed
Tracked by #251

Handle INTERVAL type in the backend #430

kgodey opened this issue Jul 18, 2021 · 5 comments · Fixed by #1067
Assignees
Labels
type: enhancement New feature or request work: backend Related to Python, Django, and simple SQL

Comments

@kgodey
Copy link
Contributor

kgodey commented Jul 18, 2021

This issue is to ensure that Mathesar can handle the Postgres INTERVAL data type.

As part of this issue, we need to ensure that:

  • Users can use the API to change a column to INTERVALif it's possible to do so (this should already be complete).
  • Users can set and change the precision and fields of a given INTERVAL column via a type_options field in the API.
  • Automatic type inference during file import suggests INTERVAL when it makes sense to do so.

Additional Context

@kgodey kgodey added needs: unblocking Blocked by other work type: enhancement New feature or request work: backend Related to Python, Django, and simple SQL work: database labels Jul 18, 2021
@kgodey
Copy link
Contributor Author

kgodey commented Jul 18, 2021

Marking as blocked until backend work in #250 is completed because they're related and we want to make sure implementations are consistent.

@silentninja
Copy link
Contributor

@kgodey Since the backend issues that block this issue have a PR, can I work on this or should I wait till the blocking frontend issues are sorted out.

@kgodey
Copy link
Contributor Author

kgodey commented Dec 8, 2021

@silentninja I think it's fine to start work on this, it was blocked by backend work.

@kgodey kgodey added ready Ready for implementation and removed needs: unblocking Blocked by other work labels Dec 9, 2021
@silentninja silentninja added status: started and removed ready Ready for implementation labels Dec 13, 2021
@silentninja silentninja self-assigned this Dec 13, 2021
@silentninja silentninja reopened this Dec 14, 2021
@silentninja silentninja removed their assignment Jan 18, 2022
@mathemancer
Copy link
Contributor

mathemancer commented Jan 24, 2022

@kgodey I don't think we should add any parsing in python for this, since the libraries you mentioned are less featureful than the native PostgreSQL parsing. Both understand fewer strings. For output and viewing, I think we should actually use strings rather than the default python timedelta. Unless we have reason to work with these in python, that is. The reason is that the timedelta in python is quite broken (it can't do months or years or decades properly). By contrast, the native PostgreSQL INTERVAL can add a decade properly (including leap days and seconds, etc.). This means the user would see a different value in the UI than would actually be added under the hood for, e.g., a month.

@kgodey
Copy link
Contributor Author

kgodey commented Jan 24, 2022

@kgodey I don't think we should add any parsing in python for this, since the libraries you mentioned are less featureful than the native PostgreSQL parsing. Both understand fewer strings. For output and viewing, I think we should actually use strings rather than the default python timedelta. Unless we have reason to work with these in python, that is. The reason is that the timedelta in python is quite broken (it can't do months or years or decades properly). By contrast, the native PostgreSQL INTERVAL can add a decade properly (including leap days and seconds, etc.). This means the user would see a different value in the UI than would actually be added under the hood for, e.g., a month.

@mathemancer and I discussed this during our check-in today and it sounds fine to me. The plan is to create a custom SQLAlchemy type to handle INTERVAL DB types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request work: backend Related to Python, Django, and simple SQL
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants