From cd9c1a33d8b972fee327ebfbc76e8912a65fa0a7 Mon Sep 17 00:00:00 2001 From: Jacek Ablewicz Date: Wed, 17 Aug 2016 11:52:48 +0200 Subject: [PATCH] [SIGNED-OFF] Bug 17138 - UpdateFine() modyfies existing fine records even when there is no need In C4/Overdues.pm: 574: # we're updating an existing fine. Only modify if amount changed 577: if ( $data->{'amount'} != $amount ) { for some numbers (e.g. 5.60) the comparison in line 577 fails ($data->{'amount'} is decimal(28,6) i.e. a string, and $amount may be a floating point number with no exact binary representation). E.g. when the fine amounts are multiple of 0.20, it fails for the 33.3% of the amounts (on average). Depending on the fine charging settings, this may trigger unnecessary database writes (and a lot of them). If 'Fine Charging Interval' is 1 day, this has very little practical consequences (misc/cronjobs/fines.pl script is usually run once per day, and all fines need an increase anyway). But if the charging interval is longer, e.g. 7 days, in a given day amount changes only for the 1/7 of the fines (again, on average), and over 60% of the database writes would be redundant. To reproduce: 1) run misc/cronjobs/fines.pl 2) add warn "AMOUNT ".$data->{'amount'}." vs $amount"; below line 577 in C4/Overdues.pm 3) run misc/cronjobs/fines.pl again 4) you'll see some warnings like 'AMOUNT 5.600000 vs 5.6' or: dump accountlines table after step 1), run the script 2nd time, dump the table again and compare the contents - some of the records would have a different timestamp. To test: 1) apply patch 2) redo steps 1) - 4) above 3) no more warnings in step 4 4) 2nd run of misc/cronjobs/fines.pl should be noticeably faster Signed-off-by: Josef Moravec --- C4/Overdues.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 8300a1e..feb4eef 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -575,7 +575,7 @@ sub UpdateFine { # we're updating an existing fine. Only modify if amount changed # Note that in the current implementation, you cannot pay against an accruing fine # (i.e. , of accounttype 'FU'). Doing so will break accrual. - if ( $data->{'amount'} != $amount ) { + if ( $data->{'amount'} ne sprintf('%.6f', $amount) ) { my $accountline = Koha::Account::Lines->find( $data->{accountlines_id} ); my $diff = $amount - $data->{'amount'}; -- 2.1.4