* Paul Poulain ([email protected]) wrote: > Le 13/04/2011 15:40, Galen Charlton a écrit : > > Hi, > > > > On Tue, Apr 12, 2011 at 9:38 PM, Srdjan <[email protected]> wrote: > >> 1. issues table has nullable borrowernumber and itemnumber; foreign > >> key constraints are consequently declared as "ON DELETE SET NULL ON > >> UPDATE SET NULL" > >> > >> 2. That is not an isolated issue, there are many cases where foreign > >> key field/constraints are declared like that > >> > >> Can anyone shed more light, that looksvery wrong to me. > > And it is very wrong; this should be tightened up. As to why? > > Reasons best hidden in the mists of time, no doubt. > no, no, they are hidden in my memory, I explain: > koha 2.2 used myISAM tables, so no contraints. > one of the first thing I did for 3.0 was to switch to innoDB and add > constraints. When I did that, all issues were in issues table (including > issues already returned) > Then someone from LibLime added the oldissues table, and wrote the code > to move issues to oldissues were applicable. > > The SET NULL *MUST BE KEPT* for statistic purposes on checked-in items : > if you delete the item, you still can have usefull informations ! > On issues, I agree that we have a problem now = it contains only > checked-out items, that must not be deleted, otherwise the issue will > never be checked-out. > So we have 3 options : > * use a trigger on item deletion, to move the issues to oldissues if > needed. But with mySQL this is not possible > * switch to ON DELETE CASCADE on item deletion, but that will loose some > information for stats (could we accept this, considering deleting an > item that has not been checked-in is very un-common ?) > * add business logic to Koha to move the issue when the item is deleted. > Isn't this already the case ? if it's already the case, then we don't > care of the constraint on mySQL schema, as there won't be anything in > issues, so the case will never happend.
I disagree, option 4 is to make the change suggested by Srdjan, in which the database will enforce integrity and make it impossible to delete an item that is checked out (has a row in issues). This covers us from mistakes in code, and from people making mistakes in the database. DELETE CASCADE is dangerous and should be avoided in this instance. It means if we make a mistake anywhere deleting an item that is checked out, the mistake will be compounded as we now delete the row in issues. The set NULL is nearly as bad, as we now null the column and still have no idea what it was linked to. > > About > > 2. That is not an isolated issue, there are many cases where foreign > > key field/constraints are declared like that > I wrote many of them with a reason that I think needed. Please tell me > which you consider as a problem, i'll tell you : > * why I choose this option if it's me > * that I was not the author of this choice, the author made a mistake > (and if git blame show you i'm the author of a buggy/wrong choice, i'll > argue that someone stole my identity just for this mistake :D ) > I think the database should never do this, I think our code should do it if it is needed. The database should stop us deleting things that are referred to elsewhere. We should clean up those references before trying to delete. The database should be the failsafe for us making mistakes, not allow us to make bigger mistakes :) Chris -- Chris Cormack Catalyst IT Ltd. +64 4 803 2238 PO Box 11-053, Manners St, Wellington 6142, New Zealand
signature.asc
Description: Digital signature
_______________________________________________ Koha-devel mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
