Bug 27511

Summary: Don't use NOW() in saved sql last run update
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: ReportsAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED INVALID QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, martin.renvoize, severine.queune
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26669
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27574
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 27573    
Attachments: Bug 27511: Don't use NOW() in saved sql last run update
Bug 27511: Don't use NOW() in saved sql last run update

Description Fridolin Somers 2021-01-21 16:00:19 UTC
When running a report, column last run is updated with current date-time.
We should avoid using NOW(), better calculated date-time in perl.

With NOW(), when importing a backup dump, you get current date-time in all rows. 

Also, sometimes we see errors in plack logs :
DBD::mysql::db do failed: Lock wait timeout exceeded; try restarting transaction [for Statement "UPDATE saved_sql SET last_run = NOW() WHERE id = ?"] at /home
/koha/src/C4/Reports/Guided.pm line 576.

Replacing NOW() should avoid this.
Comment 1 Fridolin Somers 2021-01-21 16:03:08 UTC
See that NOW() has been replaces in C4::Stats by Bug 24151
Comment 2 Fridolin Somers 2021-01-21 16:04:19 UTC
Created attachment 115565 [details] [review]
Bug 27511: Don't use NOW() in saved sql last run update

When running a report, column last run is updated with current date-time.
We should avoid using NOW(), better calculated date-time in perl.

With NOW(), when importing a backup dump, you get current date-time in all rows.

Also, sometimes we see errors in plack logs :
DBD::mysql::db do failed: Lock wait timeout exceeded; try restarting transaction [for Statement "UPDATE saved_sql SET last_run = NOW() WHERE id = ?"] at /home
/koha/src/C4/Reports/Guided.pm line 576.

Replacing NOW() should avoid this.

Test plan:
1) Run a sql report
2) Check last run date and time is OK
Comment 3 Fridolin Somers 2021-01-28 09:07:25 UTC
Or maybe we should use Koha::Report (ORM) ?
with $report->last_run($date)->store();
Comment 4 PTFS Europe Sandboxes 2021-03-31 13:58:22 UTC
Created attachment 119050 [details] [review]
Bug 27511: Don't use NOW() in saved sql last run update

When running a report, column last run is updated with current date-time.
We should avoid using NOW(), better calculated date-time in perl.

With NOW(), when importing a backup dump, you get current date-time in all rows.

Also, sometimes we see errors in plack logs :
DBD::mysql::db do failed: Lock wait timeout exceeded; try restarting transaction [for Statement "UPDATE saved_sql SET last_run = NOW() WHERE id = ?"] at /home
/koha/src/C4/Reports/Guided.pm line 576.

Replacing NOW() should avoid this.

Test plan:
1) Run a sql report
2) Check last run date and time is OK

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 5 Séverine Queune 2021-03-31 14:00:44 UTC
(In reply to Fridolin Somers from comment #3)
> Or maybe we should use Koha::Report (ORM) ?
> with $report->last_run($date)->store();

I don't have any opinion about that point, the way the patch is written works fine to me.
Comment 6 Fridolin Somers 2021-04-07 14:01:14 UTC
(In reply to Séverine Queune from comment #5)
> (In reply to Fridolin Somers from comment #3)
> > Or maybe we should use Koha::Report (ORM) ?
> > with $report->last_run($date)->store();
> 
> I don't have any opinion about that point, the way the patch is written
> works fine to me.

QA will tell ;)
Thanks a lot for testing
Comment 7 Jonathan Druart 2021-04-14 08:50:56 UTC
We had this discussion on #koha IIRC, a couple of weeks ago.

We don't think this is actually needed. The timezone we use from the DBMS and the perl code should be the same and so this patch won't have any effects and is not fixing a bug.

Feel free to reopen if you have an example of what you are trying to fix.
Comment 8 Jonathan Druart 2021-04-14 08:52:11 UTC
Additionally, I don't think replacing NOW will fix the error "Lock wait timeout exceeded". The problem must be somewhere else.
Comment 9 Fridolin Somers 2021-04-16 11:32:04 UTC
Ok no problem.

One known consequence of NOW() is that when re-import a dump, you loose the original date.
But it is not really important.