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

Deprecate datetime field type in database #223

Merged
merged 3 commits into from
Mar 12, 2018

Conversation

ikedas
Copy link
Member

@ikedas ikedas commented Mar 6, 2018

This PR deprecates datetime field type in database. Because:

  • There are no portable way to compare, increase or decrease such field values.
  • It's slow: Values should be converted to Unix timestamp; Index cannot be applied.

Database will be upgraded automatically during upgrading process.

…eted.

Instead of these datetime fields, update_epoch_admin & update_epoch_subscriber will be used to compare timestamps easily.
@ikedas ikedas added the design label Mar 6, 2018
@ikedas ikedas added this to the 6.2.26 milestone Mar 6, 2018
@racke
Copy link
Contributor

racke commented Mar 6, 2018

I have some doubts about this idea ... we would have definitely less issues with it when we are using DBIx::Class. The other problem with using Unix timestamps is they are not legible on the SQL level compared to datetime entries.

@ikedas
Copy link
Member Author

ikedas commented Mar 6, 2018

I have some doubts about this idea ... we would have definitely less issues with it when we are using DBIx::Class.

I agree to that we would be better to use smarter ORM.

The other problem with using Unix timestamps is they are not legible on the SQL level compared to datetime entries.

Honestly, I wanted to use update_date_* fields to determine either each record have to be removed or not (this is a part of changes related to issue #11), and I found there is no portable way to compare between datetime field and system clock time (we have to support four or more dialects of SQL).

I guess these fields have been used to store just a time stamp and not considered arithmetic usage I wanted.

@ikedas
Copy link
Member Author

ikedas commented Mar 7, 2018

For example I want to do such: https://git.io/vAFnj

Anyway, if there are not strong objection, I'd like to merge my PR at this time.

@ikedas ikedas merged commit a6ec373 into sympa-community:sympa-6.2 Mar 12, 2018
@ikedas ikedas deleted the deprecate_datetime_in_db branch March 12, 2018 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants