Simply put, the borrowers table column updated_on in kohastructure.sql specifies that it cannot be null, but the updatedatabase.pl it can. It appears the NOT got lost when the column as renamed 'updated_on' from 'timestamp' in a followup on bug 10459. Fixing it is easy, the question is, what do we do with currently NULL updated_on's? My though is to set them to the unix epoch ( 1970-01-01 ).
What about MAX(date_renewed, dateenrolled, last_seen)?
(In reply to Jonathan Druart from comment #1) > What about MAX(date_renewed, dateenrolled, last_seen)? +1
Created attachment 132819 [details] [review] Bug 27253: Fix definition of updated_on in borrowers and deletedborrowers Test plan: 1. First you have to be in a state where updated_on is NULL-able. You can do that by either: a) do a fresh install of Koha 16.05 and update to master, or b) execute the following SQL queries: ALTER TABLE borrowers MODIFY updated_on timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp() COMMENT 'time of last change could be useful for synchronization with external systems (among others)' ALTER TABLE deletedborrowers MODIFY updated_on timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp() COMMENT 'time of last change could be useful for synchronization with external systems (among others)' 2. Create two borrowers (let's name them X and Y) 3. Delete borrower Y 4. Set the column updated_on to NULL for both borrowers by executing the following SQL query: UPDATE borrowers SET updated_on = NULL WHERE borrowernumber = <borrowernumber of X> UPDATE deletedborrowers SET updated_on = NULL WHERE borrowernumber = <borrowernumber of Y> 5. Apply patch and run updatedatabase 6. Verify that borrowers.updated_on and deletedborrowers.updated_on are not NULL-able. Verify that updated_on for X and Y have taken the value of dateenrolled. 7. Repeat step 2 to 6 but this time renew the patron and/or log in with its account in order to set the columns borrowers.date_renewed and borrowers.lastseen before executing updatedatabase borrowers.updated_on should take the greatest value among dateenrolled, date_renewed, and lastseen
Created attachment 132934 [details] [review] Bug 27253: Fix definition of updated_on in borrowers and deletedborrowers Test plan: 1. First you have to be in a state where updated_on is NULL-able. You can do that by either: a) do a fresh install of Koha 16.05 and update to master, or b) execute the following SQL queries: ALTER TABLE borrowers MODIFY updated_on timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp() COMMENT 'time of last change could be useful for synchronization with external systems (among others)' ALTER TABLE deletedborrowers MODIFY updated_on timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp() COMMENT 'time of last change could be useful for synchronization with external systems (among others)' 2. Create two borrowers (let's name them X and Y) 3. Delete borrower Y 4. Set the column updated_on to NULL for both borrowers by executing the following SQL query: UPDATE borrowers SET updated_on = NULL WHERE borrowernumber = <borrowernumber of X> UPDATE deletedborrowers SET updated_on = NULL WHERE borrowernumber = <borrowernumber of Y> 5. Apply patch and run updatedatabase 6. Verify that borrowers.updated_on and deletedborrowers.updated_on are not NULL-able. Verify that updated_on for X and Y have taken the value of dateenrolled. 7. Repeat step 2 to 6 but this time renew the patron and/or log in with its account in order to set the columns borrowers.date_renewed and borrowers.lastseen before executing updatedatabase borrowers.updated_on should take the greatest value among dateenrolled, date_renewed, and lastseen Signed-off-by: Owen Leonard <oleonard@myacpl.org>
QA: Looking here
(In reply to Kyle M Hall from comment #0) > Simply put, the borrowers table column updated_on in kohastructure.sql > specifies that it cannot be null, but the updatedatabase.pl it can. It > appears the NOT got lost when the column as renamed 'updated_on' from > 'timestamp' in a followup on bug 10459. > > Fixing it is easy, the question is, what do we do with currently NULL > updated_on's? My though is to set them to the unix epoch ( 1970-01-01 ). (In reply to Jonathan Druart from comment #1) > What about MAX(date_renewed, dateenrolled, last_seen)? I think it is an arbitrary choice. For my part NOW() would have been good either (and much easier). After all, we are updating those borrowers .. now, right?
$DBversion = "16.06.00.002"; if ( CheckVersion($DBversion) ) { unless ( column_exists('borrowers', 'updated_on') ) { $dbh->do(q{ ALTER TABLE borrowers ADD COLUMN updated_on timestamp NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP AFTER privacy_guarantor_checkouts; }); $dbh->do(q{ ALTER TABLE deletedborrowers ADD COLUMN updated_on timestamp NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP AFTER privacy_guarantor_checkouts;
Created attachment 133342 [details] [review] Bug 27253: Fix definition of updated_on in borrowers and deletedborrowers Test plan: 1. First you have to be in a state where updated_on is NULL-able. You can do that by either: a) do a fresh install of Koha 16.05 and update to master, or b) execute the following SQL queries: ALTER TABLE borrowers MODIFY updated_on timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp() COMMENT 'time of last change could be useful for synchronization with external systems (among others)' ALTER TABLE deletedborrowers MODIFY updated_on timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp() COMMENT 'time of last change could be useful for synchronization with external systems (among others)' 2. Create two borrowers (let's name them X and Y) 3. Delete borrower Y 4. Set the column updated_on to NULL for both borrowers by executing the following SQL query: UPDATE borrowers SET updated_on = NULL WHERE borrowernumber = <borrowernumber of X> UPDATE deletedborrowers SET updated_on = NULL WHERE borrowernumber = <borrowernumber of Y> 5. Apply patch and run updatedatabase 6. Verify that borrowers.updated_on and deletedborrowers.updated_on are not NULL-able. Verify that updated_on for X and Y have taken the value of dateenrolled. 7. Repeat step 2 to 6 but this time renew the patron and/or log in with its account in order to set the columns borrowers.date_renewed and borrowers.lastseen before executing updatedatabase borrowers.updated_on should take the greatest value among dateenrolled, date_renewed, and lastseen Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
FROM_UNIXTIME(0) does not seem to work MariaDB [koha_kohadev]> update borrowers set updated_on="1970-01-01 00:00:01" where borrowernumber=1; Query OK, 1 row affected (0.011 sec) Rows matched: 1 Changed: 1 Warnings: 0 MariaDB [koha_kohadev]> update borrowers set updated_on="1970-01-01 00:00:00" where borrowernumber=1; Query OK, 1 row affected, 1 warning (0.012 sec) Rows matched: 1 Changed: 1 Warnings: 1 MariaDB [koha_kohadev]> select updated_on from borrowers where borrowernumber=1; +---------------------+ | updated_on | +---------------------+ | 0000-00-00 00:00:00 | +---------------------+ 1 row in set (0.001 sec) MariaDB [koha_kohadev]> show warnings; +---------+------+-----------------------------------------------------+ | Level | Code | Message | +---------+------+-----------------------------------------------------+ | Warning | 1264 | Out of range value for column 'updated_on' at row 1 | +---------+------+-----------------------------------------------------+ 1 row in set (0.000 sec) MariaDB [koha_kohadev]> update borrowers set updated_on="1970-01-01 00:00:01" where borrowernumber=1; Query OK, 1 row affected (0.011 sec) Rows matched: 1 Changed: 1 Warnings: 0
I get this error: ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Incorrect datetime value: '1970-01-01 00:00:00' for column `koha_kohadev`.`borrowers`.`updated_on` at row 4739 at /kohadevbox/koha/C4/Installer.pm line 738
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
(In reply to Fridolin Somers from comment #11) > Pushed to master for 22.05, thanks to everybody involved [U+1F984] Sorry I forgot yesterday to update status
May I revert ?
Hi, we recently had some issues with updated timestamps on updates. Can you confirm that this will only change records where the updated_on was NULL before?
(In reply to Katrin Fischer from comment #14) > Hi, > > we recently had some issues with updated timestamps on updates. Can you > confirm that this will only change records where the updated_on was NULL > before? + WHERE updated_on IS NULL
Reading helps - thx Marcel.
MariaDB [koha_myclone]> update borrowers set updated_on=COALESCE( NULL, FROM_UNIXTIME(0) ) where borrowernumber=51; ERROR 1292 (22007): Incorrect datetime value: '1970-01-01 00:00:00' for column `koha_myclone`.`borrowers`.`updated_on` at row 1 MariaDB [koha_myclone]> update borrowers set updated_on=COALESCE( NULL, FROM_UNIXTIME(1) ) where borrowernumber=51; Query OK, 1 row affected (0.008 sec) Rows matched: 1 Changed: 1 Warnings: 0 So I would propose to switch to UNIXTIME(1). Note that I would consider this a bug in SQL. Will add a follow-up
Created attachment 133735 [details] [review] Bug 27253: (follow-up) Fix UNIXTIME(0) in db_revs/211200037.pl MariaDB [koha_myclone]> update borrowers set updated_on=COALESCE( NULL, FROM_UNIXTIME(0) ) where borrowernumber=51; ERROR 1292 (22007): Incorrect datetime value: '1970-01-01 00:00:00' for column `koha_myclone`.`borrowers`.`updated_on` at row 1 MariaDB [koha_myclone]> update borrowers set updated_on=COALESCE( NULL, FROM_UNIXTIME(1) ) where borrowernumber=51; Query OK, 1 row affected (0.008 sec) Rows matched: 1 Changed: 1 Warnings: 0 Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested by switching updated_on to datetime. Remove NOT NULL, etc. Copied dbrev to atomicupdate folder. Resulted in: Updated all NULL values of borrowers.updated_on to GREATEST(date_renewed, dateenrolled, lastseen): 51 rows updated
(In reply to Fridolin Somers from comment #13) > May I revert ? Push the follow-up please.
(In reply to Marcel de Rooy from comment #17) > So I would propose to switch to UNIXTIME(1). > Note that I would consider this a bug in SQL. It's weird that '1970-01-01 00:00:00' is not a valid timestamp but it's not really a bug IMO. MySQL documentation says that: The TIMESTAMP data type is used for values that contain both date and time parts. TIMESTAMP has a range of '1970-01-01 00:00:01' UTC to '2038-01-19 03:14:07' UTC. https://dev.mysql.com/doc/refman/8.0/en/datetime.html
(In reply to Julian Maurice from comment #20) > (In reply to Marcel de Rooy from comment #17) > > So I would propose to switch to UNIXTIME(1). > > Note that I would consider this a bug in SQL. > > It's weird that '1970-01-01 00:00:00' is not a valid timestamp but it's not > really a bug IMO. MySQL documentation says that: > > The TIMESTAMP data type is used for values that contain both date and time > parts. TIMESTAMP has a range of '1970-01-01 00:00:01' UTC to '2038-01-19 > 03:14:07' UTC. > > https://dev.mysql.com/doc/refman/8.0/en/datetime.html Ok no bug, inconsistency then :)
(In reply to Marcel de Rooy from comment #19) > (In reply to Fridolin Somers from comment #13) > > May I revert ? > > Push the follow-up please. Done
Pushed to 21.11.x for 21.11.06
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.