(dev/core#174) civicrm_cache - Finish transition from DATETIME to TIMESTAMP #12368
Conversation
v4.7.20 updated the definition of `created_date` and `expired_date` so that new installs would default to TIMESTAMP instead of DATETIME. Status-checks and DoctorWhen have been encouraging a transition, but it wasn't mandated, and there was little urgency... because `expired_date` was ignored, and adhoc TTLs on `created_date` had generally long windows. Now that we're using `expired_date` in more important ways for 5.4 (dev/core#174), we want to ensure that these values are handled precisely and consistently. Before ------------------------------ * On new deployments, `created_date` and `expired_date` are TIMESTAMP. * On upgraded deployments, `created_date` and `expired_date` may be using DATETIME or TIMESTAMP (depending on the particular history and administrative decisions). After ------------------------------ * On all deployments, `created_date` and `expired_date` are TIMESTAMP.
|
(Standard links)
|
|
Looks fine to me makes sense, @totten i wonder will this have an impact on current caches and should caches get busted after this upgrade step so that expired and created dates are all re-generated? |
I was wondering about that too. Poking into |
|
ok i think that is ok then, That should ensure that all caches are set properly after the upgrade |
|
I agree this is safe - we have a long history of using timestamp fields where previously date time fields have been used without problems. In the case of the cache tables we are very clearly dealing with a field that should be timestamp. Merging |
Overview
v4.7.20 (4387c66) updated the definition of
civicrm_cacheso thatcreated_dateandexpired_datewould default to TIMESTAMP on new deployments. For existing deployments, status-checks and DoctorWhen have been encouraging a transition, but it wasn't mandated, and there was little urgency... becauseexpired_datewas generally ignored, and adhoc TTLs oncreated_datehad generally long windows.However, in v5.4, we'll be using
expired_datein more important ways (dev/core#174), and we should ensure that these values are handled precisely and consistently.Before
created_dateandexpired_dateare TIMESTAMP.created_dateandexpired_datemay be using DATETIME or TIMESTAMP (depending on the particular history and administrative decisions).After
created_dateandexpired_dateare TIMESTAMPs.Comments
In the interest of full disclosure -- I expect there are scenarios in which having
expired_dateasDATETIMEwill lead to bugs in 5.4+, but I haven't empirically proven it. This patch preemptively reduces the number of edge-cases we have to consider in managing cache TTLs.Generally, I'm inclined to view the change as safe because (a) all new deployments since 4.7.20 have been using this schema anyway and (b) nothing in
civicrm-corehas been usingexpired_date.CC @seamuslee001 (resident expert in timezones). Also, ping @scardinius @systopia. (Grepping
universe, I foundde.systopia.campaigndoes a few direct queries on this table - so maybe you have some thought?)