Bug 10185 - updatedatabase.pl remove all reserves
Summary: updatedatabase.pl remove all reserves
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low blocker (vote)
Assignee: Julian Maurice
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-03 15:19 UTC by Julian Maurice
Modified: 2019-06-27 09:24 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10185: Fix update 3.09.00.025 (1.32 KB, patch)
2013-05-03 15:21 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10185: Fix update 3.09.00.025 (1.38 KB, patch)
2013-05-03 17:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10185: Add missing columns in update 3.09.00.025 (2.31 KB, patch)
2013-05-06 06:45 UTC, Julian Maurice
Details | Diff | Splinter Review
[PASSED QA] Bug 10185: Add missing columns in update 3.09.00.025 (3.31 KB, patch)
2013-05-07 06:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 10185: Fix update 3.09.00.025 (1.48 KB, patch)
2013-05-07 06:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 10185: Add missing columns in update 3.09.00.025 (3.31 KB, patch)
2013-05-07 06:48 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2013-05-03 15:19:59 UTC
Update number 3.09.00.025 can remove all reserves from reserves table when there is no entries in old_reserves.
This is due to @ai which is set to NULL in
  SET @ai = ( SELECT MAX( reserve_id ) FROM tmp_reserves )
and reserve_id > NULL returns no results in
  INSERT INTO reserves SELECT * FROM tmp_reserves WHERE reserve_id > @ai

This can be fixed easily by using COALESCE
  INSERT INTO reserves SELECT * FROM tmp_reserves WHERE reserve_id > COALESCE(@ai, 0)
Comment 1 Julian Maurice 2013-05-03 15:21:11 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2013-05-03 15:30:11 UTC
Note that this bug concern version 3.10, but also 3.12 and master
Comment 3 Kyle M Hall 2013-05-03 17:18:53 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2013-05-04 08:44:12 UTC
Can you please write a test plan for this?
Comment 5 Julian Maurice 2013-05-05 08:48:24 UTC
1/ Do a fresh 3.8 install
2/ Do some reserves
3/ Check your old_reserves table is empty
4/ Update your sources to 3.10
5/ Run updatedatabase.pl
6/ Your reserves table is now empty!

Now repeat steps 1 to 4, then apply the patch and run updatedatabase.pl
Your reserves table should still contains the reserves made at step 2
Comment 6 Katrin Fischer 2013-05-05 20:06:51 UTC
Thank you, Julian!
Comment 7 Katrin Fischer 2013-05-05 21:20:37 UTC
Hi Julian,

thx for the test plan. It works as described, but I found something that is weird here - we are losing the suspended holds in the update process.
The reserves seem ok, but the suspend_until is empty after the update.
Comment 8 Katrin Fischer 2013-05-05 21:39:39 UTC
Ok, I guess I found it: suspend_until is missing from the update?

http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=installer/data/mysql/updatedatabase.pl;h=be4dfa267c465d0e039ad2f72771892dfc41cadb;hb=92f289e9a19a61ca2bd509a0f251b13de5e4c773#l5478

Could you take a look at that and add a follow up?
Comment 9 Katrin Fischer 2013-05-05 21:47:43 UTC
2 evil data loss bugs caused by "3.09.00.025" - marking this blocker.
Comment 11 Julian Maurice 2013-05-06 06:45:39 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2013-05-06 07:53:50 UTC
Good catch, Julian!
Comment 13 Katrin Fischer 2013-05-07 06:17:29 UTC
We will need this fix in all versions that have this database update - setting version to master.
Comment 14 Katrin Fischer 2013-05-07 06:45:19 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2013-05-07 06:48:31 UTC
Created attachment 17974 [details] [review]
[PASSED QA] Bug 10185: Fix update 3.09.00.025

It can remove all reserves from reserves table when there is no entries
in old_reserves.
This is due to @ai which is set to NULL in
  SET @ai = ( SELECT MAX( reserve_id ) FROM tmp_reserves )
and reserve_id > NULL returns no results in
  INSERT INTO reserves SELECT * FROM tmp_reserves WHERE reserve_id > @ai

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Test plan and comments on second patch.
Comment 16 Katrin Fischer 2013-05-07 06:48:40 UTC
Created attachment 17975 [details] [review]
[PASSED QA] Bug 10185: Add missing columns in update 3.09.00.025

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
SQL only changes fixing an important database update.

IMPORTANT: Patches must be backported to all versions containing
the database update 3.09.00.025.

I used the test plan provided by Julian with some additions:
1/ Do a fresh 3.8 install
2/ Do some reserves
3/ Check your old_reserves table is empty
4/ Update your sources to 3.10
5/ Run updatedatabase.pl
6/ Your reserves table is now empty!

I made sure I had item level and title level holds, also
suspended holds.

I repeated the test with entries in my old_reserves table
(cancelled and filled holds) to make sure this is also still
working correctly.

Before the patch we lost all suspended holds and when the
old_reserves table was empty also all holds in general.
After the patch the reserve_id is added but the other data
in both tables is not altered.

NOTE: Adding the reserve_id first to old_reserves and then
to reserves we are missing one id in between.
old_reserves: 1,2,3
reserves: 5,6
Comment 17 Katrin Fischer 2013-05-07 06:50:06 UTC
Treating the second patch as a QA follow up.
Comment 18 Jared Camins-Esakov 2013-05-08 14:01:13 UTC
This patch has been pushed to master and 3.12.x.
Comment 19 Chris Cormack 2013-05-12 03:17:40 UTC
Pushed to 3.10.x will be in 3.10.6
Comment 20 Julian Maurice 2013-06-13 08:55:23 UTC
Ok in master, 3.12.x and 3.10.x