Bug 1368

Summary: Use of FOREIGN_KEY_CHECKS = 0
Product: Koha Reporter: Chris Cormack <chris>
Component: Installation and upgrade (web-based installer)Assignee: Bugs List <koha-bugs>
Status: CLOSED WONTFIX QA Contact: Bugs List <koha-bugs>
Severity: major    
Priority: P2 CC: gmcharlt, jonathan.druart, katrin.fischer, kyle, mjr, paul.poulain, tomascohen, ztajoli
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:

Description Chris Cormack 2010-05-21 00:29:39 UTC


---- Reported by jmf@liblime.com 2007-07-02 21:10:39 ----

Running web installer, 

02_authorities_normal_marc21.sql

ERROR 1452 (23000) at line 1190: Cannot add or update a child row: a foreign key constraint fails (`koha/auth_tag_structure`, CONSTRAINT `auth_tag_structure_ibfk_1` FOREIGN KEY (`authtypecode`) REFERENCES `auth_types` (`authtypecode`) ON DELETE CASCADE ON UPDATE CASCADE)



---- Additional Comments From jmf@liblime.com 2007-07-02 22:14:43 ----

Another error, when I try the French langauge:

    * authorites_norme_unimarc.sql
      ERROR 1452 (23000) at line 680: Cannot add or update a child row: a foreign key constraint fails (`koha/auth_tag_structure`, CONSTRAINT `auth_tag_structure_ibfk_1` FOREIGN KEY (`authtypecode`) REFERENCES `auth_types` (`authtypecode`) ON DELETE CASCADE ON UPDATE CASCADE)





---- Additional Comments From mjr@ttllp.co.uk 2007-07-03 01:19:01 ----

This didn't happen for me when installing the mixed koha3.0/CVS test system on Sunday 1 July 2007.  I'll retest and make really sure I try to load that file.  Which other framework options did you choose?



---- Additional Comments From jmf@liblime.com 2007-07-03 04:16:31 ----

The web installer used to sort the files in the sql-datas directory, not only for display, but also when the files were being run ... this is necessary because auth_types must run first due to the constraints. It no longer seems to sort (it does sort them for display on the first page).

Also, it appears that someone has changed the workflow, and some systempreferences aren't inserted, especially the Version one, unless you run
updatedatabase ... this puts the user in a perpetual loop of running the installer, finishing, then running the installer again.

updatedatabase has many warnings also, which will confuse the user.
I'm not sure why it attempts to insert values which already exist.



---- Additional Comments From jmf@liblime.com 2007-07-03 07:23:21 ----

OK, I tried using Paul's database structure, and now I get:

# 02_authorities_normal_marc21.sql
ERROR 1062 (23000) at line 1: Duplicate entry '010-a' for key 1
# 01_auth_types.sql
ERROR 1062 (23000) at line 2: Duplicate entry '' for key 1 



---- Additional Comments From henridamien@koha-fr.org 2007-08-23 15:28:11 ----

Add 
SET FOREIGN_KEY_CHECKS = 0
to your mysql install files.



---- Additional Comments From mjr@ttllp.co.uk 2007-08-27 03:13:04 ----

It would be better if the installer SQL cannot cause FOREIGN KEY violations.  Maybe this is a topic to review in Friday's meeting?



---- Additional Comments From henridamien@koha-fr.org 2007-10-08 08:58:06 ----

Is this still a problem or is it fixed ?
Did you find a way to order files to store in DB ?



---- Additional Comments From mjr@ttllp.co.uk 2007-10-09 02:05:48 ----

A dependencies file is needed for the web installer and SQL files.  I will call for help on koha-devel.  Can someone familiar with the current SQL samples list how they depend on each other, please?

Meeting recording was http://koha.org/cgi-bin/logs.pl?user=&channel=%23koha&action=&text=&user-ddl=&channel-ddl=&action-ddl=&startdate=2007083108%3A50&enddate=2007083110%3A51&saveas=&search=Search



---- Additional Comments From paul.poulain@biblibre.com 2007-11-23 08:22:59 ----

mmm... thinking of it, I suspect we can close this bug. The sample given can easily be solved by reordering the file (marc_tag's before marc_subfield's)

And generally, if we want to add a file that need another one to be loaded, just give it a name that make him appear after the 'mother' values.
For example : sample patrons, that need patron categories and branch to be defined.

Or just set a SET FOREIGN_KEY_CHECKS = 0 if you can't.

I close the bug, if someone want to reopen it, he can, but he must also set himself as assignee ;-)



---- Additional Comments From mjr@ttllp.co.uk 2007-11-23 08:47:21 ----

Great that you think "it can be fixed by ..." - but has it been fixed?

And, once again, SET FOREIGN_KEY_CHECKS = 0 is not a fix for this bug.

Leaving assignee as-is because it's a web installer bug.

Repeating request: Can someone familiar with the current SQL samples list
how they depend on each other, please?



---- Additional Comments From henridamien@koha-fr.org 2007-11-23 09:15:21 ----

It seems from what I have read in install.pl
a) foreign keys check is disabled while adding frameworks.
b) it now orders frameworks added on name as in display.

So, both solutions have been implemented.
Is there still a problem ?





---- Additional Comments From jmf@liblime.com 2007-11-24 07:02:53 ----

hdl: "SET FOREIGN_KEY_CHECKS = 0 is not a fix for this bug"! So yes, it is still a problem.



---- Additional Comments From henridamien@koha-fr.org 2007-11-25 11:26:30 ----

are frameworks not loaded in the alphabetical order as code is supposed to do ?
If so then problem is solved, isn't it ?




---- Additional Comments From jmf@liblime.com 2007-11-25 22:48:36 ----

If the problem is solved, you should be able to remove all instances of 'SET FOREIGN_KEY_CHECKS = 0' and the installer should work properly with no errors. Otherwise, it's not solved



---- Additional Comments From gmcharlt@gmail.com 2007-12-24 14:39:16 ----

I have submitted a patch to remove use of SET FOREIGN_KEY_CHECKS = 1 from the web installer and its SQL scripts.  I have verified that the current English and French MARC21 and UNIMARC script, plus optional scripts, all load without error.

Dependencies between SQL scripts that I'm aware of include (all paths relative to installer/data/mysql):

en/optional/sample_patron.sql depends on
  * en/optional/patron_categories.sql
  * en/optional/sample_libraries.sql

fr/3-LecturePub/sample_itemtypes.sql excludes
  * fr/4-Conservation/sample_itemtypes.sql

fr/3-LecturePub/sample_itemtypes.sql excludes
  * fr/4-Conservation/sample_itemtypes.sql

Even if my patch is accepted, the bug should remain open, albeit at a lower severity, because the installer should handle cases where the user does not select *all* or *no* optional scripts, and thus needs to be smarter about dependencies.  For example, nothing currently prevents the user from trying to load sample patrons without loading the sample categories and libraries.



---- Additional Comments From gmcharlt@gmail.com 2007-12-24 14:40:06 ----

To correct my previous comment, my patch removes use of SET FOREIGN_KEY_CHECKS = *0*.



---- Additional Comments From jmf@liblime.com 2007-12-25 09:13:41 ----

I concur ... de-prioritizing



---- Additional Comments From paul.poulain@biblibre.com 2008-05-20 08:17:26 ----

continuing to order priority of normal bugs



---- Additional Comments From nengard@gmail.com 2009-12-23 17:58:39 ----

This bug hasn't been touched in nearly a year, please revisit and test and close if appropriate.



---- Additional Comments From mjr@ttllp.co.uk 2009-12-23 20:38:12 ----

REOPEN.  This bug looks like it might have crept back into two places in today's HEAD:-

data/mysql/kohastructure.sql:14:/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;
data/mysql/labels_upgrade.pl:32:    SET FOREIGN_KEY_CHECKS = 0;




---- Additional Comments From paul.poulain@biblibre.com 2009-12-24 13:53:01 ----

MJ, I think you can't avoid this one :
data/mysql/kohastructure.sql:14:/*!40014 SET
@OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;
 when you start your database.

Although I didn't checked it, so I may be wrong



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:29 UTC  ---

This bug was previously known as _bug_ 1368 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=1368

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was jmf@liblime.com.
CC member mjr@ttllp.co.uk does not have an account here

Comment 1 MJ Ray (software.coop) 2010-07-07 19:27:07 UTC
Restoring where "Previous assignee was mjr@ttllp.co.uk"
Comment 2 Chris Cormack 2012-04-07 21:53:33 UTC
Yep, having it in the kohastructure.sql makes sense, but not in updatedatabase.pl

It is in 3 places in updatedatabase.pl
Comment 3 Katrin Fischer 2015-02-12 10:21:47 UTC
This bug hasn't been touched in almost 3 years. I am resetting the asignee to default - MJ, please undo if you plan on working on this.
Comment 4 MJ Ray (software.coop) 2015-02-12 12:28:04 UTC
Please take it with my blessing. It looks like it either dates from my time as default assignee for the installer or from an overbroad post-migration reset.
Comment 5 Katrin Fischer 2015-06-07 11:59:17 UTC
This is currently the oldest open bug report, filed in May 2010. 

A quick git grep for "SET FOREIGN_KEY_CHECKS" reveals a lot of occurrences (see below)

- Do we intend on fixing those? Or should this be a WONTFIX?
- What is the goal here? Should we move some of the ideas from here to new more specific bug reports?


installer/data/mysql/de-DE/optional/sample_creator_data.sql:/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
installer/data/mysql/en/optional/sample_creator_data.sql:/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
installer/data/mysql/es-ES/optional/sample_creator_data.sql:/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
installer/data/mysql/it-IT/marcflavour/unimarc/necessari/authority_framework.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/marcflavour/unimarc/necessari/authority_framework.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/marcflavour/unimarc/necessari/unimarc_framework.sql:SET FOREIGN_KEY_CHECKS=0; 
installer/data/mysql/it-IT/marcflavour/unimarc/necessari/unimarc_framework.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/auth_val.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/auth_val.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/class_sources.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/class_sources.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/message_transport_types.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/message_transport_types.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/sample_creator_data.sql:/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
installer/data/mysql/it-IT/necessari/sample_holidays.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/sample_holidays.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/sample_itemtypes.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/sample_itemtypes.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/sample_notices_message_attributes.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/sample_notices_message_attributes.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/sample_notices_message_transports.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/sample_notices_message_transports.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/stopwords.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/stopwords.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/it-IT/necessari/userflags.sql:SET FOREIGN_KEY_CHECKS=0;
installer/data/mysql/it-IT/necessari/userflags.sql:SET FOREIGN_KEY_CHECKS=1;
installer/data/mysql/kohastructure.sql:/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
installer/data/mysql/labels_upgrade.pl:    SET FOREIGN_KEY_CHECKS = 0;
installer/data/mysql/nb-NO/2-Valgfritt/sample_creator_data.sql:/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
installer/data/mysql/patroncards_upgrade.pl:    SET FOREIGN_KEY_CHECKS = 0;
installer/data/mysql/updatedatabase.pl:    $dbh->do('SET FOREIGN_KEY_CHECKS=0 ');
installer/data/mysql/updatedatabase.pl:    $dbh->do('SET FOREIGN_KEY_CHECKS=1 ');
installer/data/mysql/updatedatabase.pl:    $dbh->do('SET FOREIGN_KEY_CHECKS=0 ');
installer/data/mysql/updatedatabase.pl:    $dbh->do('SET FOREIGN_KEY_CHECKS=1 ');
installer/data/mysql/updatedatabase.pl:    $dbh->do("SET FOREIGN_KEY_CHECKS=0 ");
installer/data/mysql/updatedatabase.pl:    $dbh->do("SET FOREIGN_KEY_CHECKS=1 ");
misc/maintenance/fix_mysql_constraints.pl:    $dbh->do("SET FOREIGN_KEY_CHECKS=0");
misc/migration_tools/22_to_30/phrase_log.sql:/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
misc/migration_tools/bulkmarcimport.pl: $dbh->do("SET FOREIGN_KEY_CHECKS = 0");
misc/migration_tools/bulkmarcimport.pl: $dbh->do("SET FOREIGN_KEY_CHECKS = 1");
Comment 6 Paul Poulain 2015-06-08 06:08:35 UTC
I think we can wontfix it.
There are some lines that are valid : the foreign_key_check=0 is necessary when you restore a dump : you don't want koha to check fk, because 1- sometimes you're temporary setting invalid data and 2- it slows the dump restore.
Maybe there are some that are non necessary, but they're harmless. So wontfix is fair

(and thanks for reviving those oldies !!!)
Comment 7 Zeno Tajoli 2015-06-08 10:16:12 UTC
Hi,

many files with FOREIGN_KEY_CHECKS = 0 / FOREIGN_KEY_CHECKS = 1 are from it-IT.

We insert in this way beacause we use then also to restore in working installations. And w e need to temporary setting invalid data.

Ok on wont'fix
Comment 8 MJ Ray (software.coop) 2015-06-08 10:35:21 UTC
I'm still not comfortable with wontfix because this could be avoided by correct ordering of the database files... it's lazy programming to turn key-checking off, isn't it?

However, it's more dangerous in updates than the installer, because it's easier to test that the installer won't leave a broken/inconsistent database.  If you want to wontfix this about the installer and then create bugs for the updates switching off key-checks, then that would be OK in my opinion.
Comment 9 Katrin Fischer 2015-06-08 10:45:43 UTC
I would be quite uncomfortable about touching those old updatedatabase statementements. They all belong to the 'new acquisition module' that we introduced prior to 3.2 release and are rather complicated:

$DBversion = '3.01.00.073'
"Upgrade to $DBversion done (adding aqcontract table)\n";

$DBversion = '3.01.00.076';
"Upgrade to $DBversion done (adding basketgroups)\n";

$DBversion = '3.01.00.076';
"Upgrade to $DBversion done (Adding new aqbudgetperiods, aqbudgets and aqbudget_planning tables  )\n";

I think it's not worth risking to break those. And they would be really hard to test, as we probably can't find a database with acq data that old easily.
Comment 10 Paul Poulain 2015-06-08 12:07:46 UTC
(In reply to Katrin Fischer from comment #9)
> I think it's not worth risking to break those. And they would be really hard
> to test, as we probably can't find a database with acq data that old easily.
+1