[Bug 18003] New: Should deletedborrowers.borrowernumber be a primary key?
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 Bug ID: 18003 Summary: Should deletedborrowers.borrowernumber be a primary key? Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 17782 (Raised from bug 17782) DBIC needs a primary key to work correctly. We could be tempt to make borrowernumber a primary key, but a use case could bring thing. Indeed if a table is empty and the mysql (mariadb behaves identically) server restarted, the Auto Increment (AI) is reset. Problematic use case: 1. Install Koha 2. Create a patron (borrowers.borrowernumber = 1) 3. Delete it (deletedborrowers.borrowernumer = 1, and AI is set to 2 4. Restart mysql => borrowers is empty so AI is reset 5. Create a patron (borrowers.borrowernumber = 1) 6. Delete it => We will try to insert a new row with an existing borrowernumber (1) into the deletedborrowers table: BOOM Solution 1: Do not care about this problematic use case and prevent to delete the last patron. Solution 2: Add a AI primary key (id) to deletedborrowers => The 2 tables borrowers and deletedborrowers will differ, bugs will be expected Solution 3: Deal with that at code level: when a patron is created, guess the borrowernumber it will get and check if it does not already exist in the deletedborrowers table. If so, force it before inserting it Solution 4: Do not enforce this constraint at DB level but set the primary key to DBIC schema: Koha::Schema::Result::Deletedborrower: __PACKAGE__->set_primary_key("borrowernumber"); Problem: Koha::Database->new->schema->resultset('Deletedborrower')->find(42); will raise a warning "DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single" if 42 is duplicated, and the first matching row will be picked Another solution? Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17782 [Bug 17782] Patron updated_on field should be set to current timestamp when borrower is deleted -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alex.sassmannshausen@ptfs-e | |urope.com, | |fridolin.somers@biblibre.co | |m, | |julian.maurice@biblibre.com | |, katrin.fischer@bsz-bw.de, | |kyle@bywatersolutions.com, | |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 59616 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59616&action=edit Bug 18003: Example of why we would need a PK on deletedborrowers These tests do not pass: DBIx::Class::ResultSource::_pri_cols_or_die(): Operation requires a primary key to be declared on 'Deletedborrower' via set_primary_key at t/db_dependent/Koha/Patrons.t line 329 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com --- Comment #2 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Jonathan Druart from comment #0)
Solution 1: Do not care about this problematic use case and prevent to delete the last patron.
I do not like this - it escalates the internal technical problem to end user...
Solution 2: Add a AI primary key (id) to deletedborrowers => The 2 tables borrowers and deletedborrowers will differ, bugs will be expected
It could be possible I think, but probably not easy to make it
Solution 3: Deal with that at code level: when a patron is created, guess the borrowernumber it will get and check if it does not already exist in the deletedborrowers table. If so, force it before inserting it
A bit complicated, but probably the best one
Solution 4: Do not enforce this constraint at DB level but set the primary key to DBIC schema: Koha::Schema::Result::Deletedborrower: __PACKAGE__->set_primary_key("borrowernumber"); Problem: Koha::Database->new->schema->resultset('Deletedborrower')->find(42); will raise a warning "DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single" if 42 is duplicated, and the first matching row will be picked
I do not like this too
Another solution?
So I am for 2 or 3, 2 was what came first to my mind, but the 3 is better I think... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #3 from Josef Moravec <josef.moravec@gmail.com> ---
Another solution?
Merge borrowers and deletedborrowers to one table and mark the deleted one with a flag -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Does the problem mentioned on top only occur on an empty borrowers table with a mysql restart? Supposing that this is the case and it is quite rare, can't we just use borrowernumber as the primary key on borrowers and deletedborrowers, obviously having no constraints on deletedborrowers. We could even add a check for borrowernumber when creating a patron, and update the borrowernumber if needed to not match deletedborrowers? Ugly, but fixing some problems. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- We discussed if it also happens when you delete the last added borrower and then restart - not sure what the answer was. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Katrin Fischer from comment #5)
We discussed if it also happens when you delete the last added borrower and then restart - not sure what the answer was.
Indeed this is a case that sucks as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- URL| |http://lists.koha-community | |.org/pipermail/koha-devel/2 | |016-January/042207.html Status|NEW |In Discussion --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- See also "Merge borrowers and deletedborrowers tables" topic on koha-devel 1 year ago: http://lists.koha-community.org/pipermail/koha-devel/2016-January/042207.htm... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #7)
See also "Merge borrowers and deletedborrowers tables" topic on koha-devel 1 year ago: http://lists.koha-community.org/pipermail/koha-devel/2016-January/042207.htm...
We don't provide a way to recover a deleted borrower, do we? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'd +1 the merging of tables.. but can see the annoyance it would cause people who have written reports using said tables.. was that the reason it was not done a year ago? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #9)
I'd +1 the merging of tables.. but can see the annoyance it would cause people who have written reports using said tables.. was that the reason it was not done a year ago?
Can't we creaste a view for those? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I like the idea of creating a view.. but the only method that would work seamlessly would be to create an entirely new 'borrowers' table with a new name and create a view for each of the old borrowers and deletedborrowers tables. Not a massive issue, but worth thinking about.. I kinda like the idea of 'doing it properly, and using views to maintain backwards compatibility with old reports' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #10)
(In reply to Martin Renvoize from comment #9)
I'd +1 the merging of tables.. but can see the annoyance it would cause people who have written reports using said tables.. was that the reason it was not done a year ago?
Can't we creaste a view for those?
For the info: I have created a view on bug 17835 (different need, different problem, but it is worth noting). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=18185 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=18242 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=18931 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 claire.hernandez@biblibre.com <claire.hernandez@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18081 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18081 [Bug 18081] [omnibus] GDPR (General Data Protection Regulation) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 George Williams (NEKLS) <george@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |george@nekls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18003 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=20271 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org