The table old_issues has a primary key defined on the issue_id column. This issue_id comes from the issues table when an item is checked in. In some case the value of issue_id already exists in the table Basically this happens when an item is returned and mysqld is restarted: The auto increment value for issues.issue_id will be reset to MAX(issue_id)+1 (which is the value of the last entry of old_issues). See also the description of bug 18003 for more informations.
Created attachment 60969 [details] [review] Bug 18242: [SOLUTION 1]DB changes - remove PK on old_issues.issue_id
Created attachment 60970 [details] [review] Bug 18242: [SOLUTION 1]Schema changes
Created attachment 60971 [details] [review] Bug 18242: [SOLUTION 1]Remove primary key on old_issues.issue_id The table old_issues has a primary key defined on the issue_id column. This issue_id comes from the issues table when an item is checked in. In some case the value of issue_id already exists in the table Basically this happens when an item is returned and mysqld is restarted: The auto increment value for issues.issue_id will be reset to MAX(issue_id)+1 (which is the value of the last entry of old_issues). See also the description of bug 18003 for more informations. The easier solution seems to add a new column id as a primary key. The changes to the codebase are very small. However this will raise few problems: - We can no longer assume that accountlines.issue_id==old_issues.id (I guess it is problematic). - Code in Koha::REST::V1::_object_ownership_by_checkout_id is wrong assuming that old_issues.issuing_id is unique
Created attachment 60972 [details] [review] Bug 18242: [SOLUTION 2]Add tests
Created attachment 60973 [details] [review] Bug 18242: [SOLUTION 2]Handle correctly move to old_issues The table old_issues has a primary key defined on the issue_id column. This issue_id comes from the issues table when an item is checked in. In some case the value of issue_id already exists in the table Basically this happens when an item is returned and mysqld is restarted: The auto increment value for issues.issue_id will be reset to MAX(issue_id)+1 (which is the value of the last entry of old_issues). See also the description of bug 18003 for more informations. In this solution the change is done at code level instead of DB structure: If old_issues.issue_id already exists before moving from the issues table, the issue_id is updated (not on cascade for accountlines.issue_id, should it?) before the move.
Created attachment 61056 [details] [review] Bug 18242: [SOLUTION 2]Add tests Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 61057 [details] [review] Bug 18242: [SOLUTION 2]Handle correctly move to old_issues The table old_issues has a primary key defined on the issue_id column. This issue_id comes from the issues table when an item is checked in. In some case the value of issue_id already exists in the table Basically this happens when an item is returned and mysqld is restarted: The auto increment value for issues.issue_id will be reset to MAX(issue_id)+1 (which is the value of the last entry of old_issues). See also the description of bug 18003 for more informations. In this solution the change is done at code level instead of DB structure: If old_issues.issue_id already exists before moving from the issues table, the issue_id is updated (not on cascade for accountlines.issue_id, should it?) before the move. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
*** Bug 18185 has been marked as a duplicate of this bug. ***
Created attachment 61254 [details] [review] Bug 18242: [SOLUTION 2]Add tests Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 61255 [details] [review] Bug 18242: [SOLUTION 2]Handle correctly move to old_issues The table old_issues has a primary key defined on the issue_id column. This issue_id comes from the issues table when an item is checked in. In some case the value of issue_id already exists in the table Basically this happens when an item is returned and mysqld is restarted: The auto increment value for issues.issue_id will be reset to MAX(issue_id)+1 (which is the value of the last entry of old_issues). See also the description of bug 18003 for more informations. In this solution the change is done at code level instead of DB structure: If old_issues.issue_id already exists before moving from the issues table, the issue_id is updated (not on cascade for accountlines.issue_id, should it?) before the move. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This works in all testing I tried, it does seem that if things get out of sync the ids will be updated for most returns and never get back into sync, however, I can't think of any reason issue_id needs to be static so seems fine
Pushed to Master - Should be in the 17.05 release. Thanks!
Patches don't appear to work for 16.11.x: # Failed test 'No tests run for subtest "Handle ids duplication"' # at t/db_dependent/Circulation/Returns.t line 289. Can't locate object method "find" via package "Koha::Old::Checkouts" (perhaps you forgot to load "Koha::Old::Checkouts"?) at t/db_dependent/Circulation/Returns.t line 287. If this is needed, please provide patches for 16.11.x!
For 16.11.x you just need to replace Koha::Old::Checkout with Koha::OldIssue (bug 17796).
Jonathan, could you please provide patches for 16.11.x?
Created attachment 61794 [details] [review] Bug 18242: 16.11.x adaptation - Old::Checkouts vs OldIssues
Please I have problems in the loans, I deleted the circulation history, please in what files should I make the changes ?. What change should I make in the database ?. I have version 16.11.06
These patches have been pushed to 16.11.x and will be in 16.11.07. Thx for the follow-up, Jonahtan!
Hi, the atches don't appear to work for 16.05.x: If this is needed, please provide patches for 16.05.x ----------- mason@xen1:~/g/k/16.05.x$ git bz apply 18242 Bug 18242 - Remove primary key on old_issues.issue_id 61254 - Bug 18242: [SOLUTION 2]Add tests 61255 - Bug 18242: [SOLUTION 2]Handle correctly move to old_issues 61794 - Bug 18242: 16.11.x adaptation - Old::Checkouts vs OldIssues Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 18242: [SOLUTION 2]Add tests Using index info to reconstruct a base tree... M t/db_dependent/Circulation/Returns.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Circulation/Returns.t CONFLICT (content): Merge conflict in t/db_dependent/Circulation/Returns.t Failed to merge in the changes. Patch failed at 0001 Bug 18242: [SOLUTION 2]Add tests The copy of the patch that failed is found in: /home/mason/g/k/16.05.x/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-18242-SOLUTION-2Add-tests-uyNJwm.patch
Created attachment 63168 [details] [review] Bug 18242: 16.05 version - [SOLUTION 2]Add tests
Created attachment 63169 [details] [review] Bug 18242: 16.05 version - [SOLUTION 2]Handle correctly move to old_issues The table old_issues has a primary key defined on the issue_id column. This issue_id comes from the issues table when an item is checked in. In some case the value of issue_id already exists in the table Basically this happens when an item is returned and mysqld is restarted: The auto increment value for issues.issue_id will be reset to MAX(issue_id)+1 (which is the value of the last entry of old_issues). See also the description of bug 18003 for more informations. In this solution the change is done at code level instead of DB structure: If old_issues.issue_id already exists before moving from the issues table, the issue_id is updated (not on cascade for accountlines.issue_id, should it?) before the move.
Created attachment 63170 [details] [review] Bug 18242: 16.05 version - 16.11.x adaptation - Old::Checkouts vs OldIssues
Last 3 patches are for 16.05.x I DID NOT TEST THEM! If you want them for 16.05.x, please test.
(In reply to Jonathan Druart from comment #23) > Last 3 patches are for 16.05.x > I DID NOT TEST THEM! > If you want them for 16.05.x, please test. thanks Jonathan, the patchset looks to be working as expected on 16.05.x :)
Pushed to 16.05.x, for 16.05.12 release
Updating the bug report title, we did not remove the PK on old_issues.
(In reply to Jonathan Druart from comment #21) > In this solution the change is done at code level instead of DB > structure: If old_issues.issue_id already exists before moving from > the issues table, the issue_id is updated (not on cascade for > accountlines.issue_id, should it?) before the move. See bug 18651 for a follow-up
Tests fail for me with and without this patch: 1..26 ok 1 - connect to intranet ok 2 - login to staff client ok 3 - load main page ok 4 - open tools module Link not found at t/db_dependent/www/batch.t line 69. # Looks like your test exited with 255 just after 4.
This patch cause a big problem for me in 16.11. The way the new issue_id are calculated/updated didn't take into consideration the case that 2 checkouts for the same item happen at the same time. While it is very unlikely, I think it shouldn't cause a problem. In our case, our Self-return vendor make a mistake and issue 2 SIP checkin messages at the same time. One of them work properly, but the other one incorrectly move the latest row in issues into old_issues. This cause the item to lost it checkout status (almost). Probably we should backport #18966 to oldstable too?
Oh, I just notice that 16.11.10 was just released and it includes #18966 too :)
(In reply to Pongtawat from comment #30) > Oh, I just notice that 16.11.10 was just released and it includes #18966 too > :) \o/
This patch does not always work for 16.11 When you change issue_id with max(issue_id)+1 from old_issues but try to insert it into issues, you may have a primary key conflict. my $new_issue_id = $dbh->selectrow_array(q|SELECT MAX(issue_id)+1 FROM old_issues|); $dbh->do( q|UPDATE issues SET issue_id = ? WHERE issue_id = ?|, undef, $new_issue_id, $issue_id );
(In reply to Sophie MEYNIEUX from comment #32) > This patch does not always work for 16.11 Which versions of 16.11.x? You should read the description of bug 18966.