Bug 17324

Summary: branchcode is NULL in letter triggers red upgrade message
Product: Koha Reporter: M. Tompsett <mtompset>
Component: Installation and upgrade (web-based installer)Assignee: M. Tompsett <mtompset>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: brendan, chris, frederic, gmcharlt, jonathan.druart, julian.maurice, ztajoli
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17292
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17260
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17234
Change sponsored?: --- Patch complexity: ---
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 17258    
Attachments: Bug 17324: Deal with branchcode NULL issue
Bug 17324: Deal with branchcode NULL issue
Bug 17324: Deal with branchcode NULL issue
Bug 17324: Deal with branchcode NULL issue

Description M. Tompsett 2016-09-20 23:31:47 UTC
I suspect there is probably relevant discussion on bug 14839.
However, I just want a clean upgrade for MySQL 5.7 on Ubuntu 16.04 LTS.

[Tue Sep 20 19:08:10 2016] updatedatabase.pl: DBD::mysql::db do failed: Invalid use of NULL value [for Statement "
[Tue Sep 20 19:08:10 2016] updatedatabase.pl: ALTER TABLE letter MODIFY COLUMN branchcode varchar(10) NOT NULL DEFAULT ''
[Tue Sep 20 19:08:10 2016] updatedatabase.pl: "] at /home/mtompset/kohaclone/installer/data/mysql/updatedatabase.pl line 12638.

Reproduce:
on an ubuntu 16.04 LTS machine with MySQL 5.7 install git koha development environment.
...
git checkout -b my_3.18.x origin/3.18.x
drop database koha_library;
create database koha_library;
quit;
web install all sample data
(ignore some issues with sample data, and one with mandatory)
git checkout -b bug_17234 origin/master
git bz apply 17292
git bz apply 17320
git bz apply 17260
git bz apply 17234
web install
Two warning messages are expected output from upgrade process, but the above message is not.
repeat the above, but git bz apply this patch before running the second web install. The message should be not there.
Comment 1 M. Tompsett 2016-09-20 23:35:59 UTC
Created attachment 55727 [details] [review]
Bug 17324: Deal with branchcode NULL issue
Comment 2 M. Tompsett 2016-09-20 23:44:20 UTC
Created attachment 55728 [details] [review]
Bug 17324: Deal with branchcode NULL issue
Comment 3 M. Tompsett 2016-09-20 23:50:45 UTC
Okay, I was wrong about the bug 14839. I vaguely recall the cascading of deletion with '' vs NULL being an issue for some reason when I was working on news by branchcode, but I don't remember the scenario.

Follow the test plan in the description. You'll get a "pristine" (two messages about a system preference that gets deleted later) install.
Comment 4 Chris Cormack 2016-10-04 21:42:59 UTC
Created attachment 56041 [details] [review]
Bug 17324: Deal with branchcode NULL issue

In a database update we alter the letter table, making a column
(branchcode) not be able to contain NULL values.
However if it already does, this SQL fails. So before changing the
column we change the NULL values

[Tue Sep 20 19:08:10 2016] updatedatabase.pl: DBD::mysql::db do failed:
Invalid use of NULL value [for Statement "
[Tue Sep 20 19:08:10 2016] updatedatabase.pl: ALTER TABLE letter MODIFY
COLUMN branchcode varchar(10) NOT NULL DEFAULT ''
[Tue Sep 20 19:08:10 2016] updatedatabase.pl: "] at
/home/mtompset/kohaclone/installer/data/mysql/updatedatabase.pl line
12638.

Reproduce:
on an ubuntu 16.04 LTS machine with MySQL 5.7 install git koha
development environment.
...
git checkout -b my_3.18.x origin/3.18.x
drop database koha_library;
create database koha_library;
quit;
web install all sample data
(ignore some issues with sample data, and one with mandatory)

git checkout -b bug_17234 origin/master
git bz apply 17292
git bz apply 17320
git bz apply 17260
git bz apply 17234
web install
Two warning messages are expected output from upgrade process, but
the above message is not.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 5 Chris Cormack 2016-10-04 21:43:23 UTC
Signed off and fixed the commit message.
Comment 6 Jonathan Druart 2016-10-05 07:21:57 UTC
Created attachment 56049 [details] [review]
Bug 17324: Deal with branchcode NULL issue

In a database update we alter the letter table, making a column
(branchcode) not be able to contain NULL values.
However if it already does, this SQL fails. So before changing the
column we change the NULL values

[Tue Sep 20 19:08:10 2016] updatedatabase.pl: DBD::mysql::db do failed:
Invalid use of NULL value [for Statement "
[Tue Sep 20 19:08:10 2016] updatedatabase.pl: ALTER TABLE letter MODIFY
COLUMN branchcode varchar(10) NOT NULL DEFAULT ''
[Tue Sep 20 19:08:10 2016] updatedatabase.pl: "] at
/home/mtompset/kohaclone/installer/data/mysql/updatedatabase.pl line
12638.

Reproduce:
on an ubuntu 16.04 LTS machine with MySQL 5.7 install git koha
development environment.
...
git checkout -b my_3.18.x origin/3.18.x
drop database koha_library;
create database koha_library;
quit;
web install all sample data
(ignore some issues with sample data, and one with mandatory)

git checkout -b bug_17234 origin/master
git bz apply 17292
git bz apply 17320
git bz apply 17260
git bz apply 17234
web install
Two warning messages are expected output from upgrade process, but
the above message is not.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Did not follow the test plan, but the changes make sense
Comment 7 Jonathan Druart 2016-10-05 07:22:35 UTC
Mark, did you get how it happened?
Comment 8 M. Tompsett 2016-10-05 15:37:22 UTC
(In reply to Jonathan Druart from comment #7)
> Mark, did you get how it happened?

Ah. I think I figured out how. I got it to happen as a result of tweaking PRIMARY to UNIQUE (see bug 17234) as part of upgrade testing. Whereas, the preferred option, by other newer patches, was to change NULL to ''. NULL cascades nicely on relationships, '' does not in my experience.
Comment 9 M. Tompsett 2016-10-05 16:17:07 UTC
I also just attempted playing around with upgrading and downgrading MySQL.
http://repo.mysql.com/
It would seem that upgrading your MySQL to 5.7 isn't likely to encounter this problem. Anyways, this is explicit, rather than assuming MySQL's implicit tweaking of NULL to ''.
Comment 10 Brendan Gallagher 2016-10-10 10:16:36 UTC
Pushed to Master - Should be in the November Release 16.11 - Thanks!
Comment 11 Frédéric Demians 2016-10-22 08:18:56 UTC
Pushed in 16.05. Will be in 16.05.05.
Comment 12 Julian Maurice 2016-10-24 09:38:48 UTC
Pushed to 3.22.x, will be in 3.22.12