Bug 17260 - updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns
Summary: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (command-line installer) (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Blou
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 17258
  Show dependency treegraph
 
Reported: 2016-09-06 16:05 UTC by Blou
Modified: 2017-05-15 10:13 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17260 - updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns (2.90 KB, patch)
2016-09-06 16:12 UTC, Blou
Details | Diff | Splinter Review
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns (4.90 KB, patch)
2017-01-27 17:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns (4.95 KB, patch)
2017-02-21 17:40 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns (4.96 KB, patch)
2017-02-22 15:04 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns (5.03 KB, patch)
2017-02-24 08:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17260: Update existing installs (907 bytes, patch)
2017-02-24 08:09 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Blou 2016-09-06 16:05:37 UTC
See also Bug 17234

The loading of file admin/searchengine/elasticsearch/mappings.yaml specifies 'type' as empty, which fails with Mysql 5.7+ which is more stringent.

Also, forcing an empty value into a boolean also fails.

Both issues arise when updatedatabase.pl hit 3.23.00.050.
Comment 1 Blou 2016-09-06 16:12:51 UTC
Created attachment 55263 [details] [review]
Bug 17260 - updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

The loading of file admin/searchengine/elasticsearch/mappings.yaml specifies 'type' as empty, which fails with Mysql 5.7+ which is more stringent.

Also, forcing an empty value into a boolean also fails.

Both issues arise when updatedatabase.pl hit 3.23.00.050.

NOTE: both issues could also be resolved by actually setting values in the load file.
      This doesn't make this solution incorrect, though.

To Test/reproduce:
   -1) Happens with Mysql 5.7.4.  Maybe earlier, but certainly at that point.
   0) Find a database on 3.22 or earlier, save it.
   1) Set your code base to master)
   2) run updatedatabase.pl
   3) See the errors on 3.23.00.050
   4) Apply the patch
   5) Reload the 3.22 db.
   6) succeed with updatedatabase.pl
Comment 2 M. Tompsett 2016-09-20 01:27:45 UTC
Comment on attachment 55263 [details] [review]
Bug 17260 - updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

Review of attachment 55263 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/updatedatabase.pl
@@ +12280,4 @@
>               `id` int(11) NOT NULL AUTO_INCREMENT, 
>               `name` varchar(255) NOT NULL COMMENT 'the name of the field as it will be stored in the search engine',
>               `label` varchar(255) NOT NULL COMMENT 'the human readable name of the field, for display', 
> +             `type` ENUM('', 'string', 'date', 'number', 'boolean', 'sum') NOT NULL COMMENT 'what type of data this holds, relevant when storing it in the search engine',

I'm not sure how to test for this.

@@ +12337,4 @@
>              my $search_field = Koha::SearchFields->find_or_create({ name => $field_name, label => $field_label, type => $field_type }, { key => 'name' });
>              for my $mapping ( @$mappings ) {
>                  my $marc_field = Koha::SearchMarcMaps->find_or_create({ index_name => $index_name, marc_type => $mapping->{marc_type}, marc_field => $mapping->{marc_field} });
> +                $search_field->add_to_search_marc_maps($marc_field, { facet => $mapping->{facet} || 0, suggestible => $mapping->{suggestible} || 0, sort => $mapping->{sort} } );

I can voucher for this change easily. I reproduced it, and this fixes it.
Comment 3 Jonathan Druart 2016-10-03 10:28:49 UTC
(In reply to M. Tompsett from comment #2)
> Comment on attachment 55263 [details] [review] [review]
> Bug 17260 - updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN
> columns
> 
> Review of attachment 55263 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: installer/data/mysql/updatedatabase.pl
> @@ +12280,4 @@
> >               `id` int(11) NOT NULL AUTO_INCREMENT, 
> >               `name` varchar(255) NOT NULL COMMENT 'the name of the field as it will be stored in the search engine',
> >               `label` varchar(255) NOT NULL COMMENT 'the human readable name of the field, for display', 
> > +             `type` ENUM('', 'string', 'date', 'number', 'boolean', 'sum') NOT NULL COMMENT 'what type of data this holds, relevant when storing it in the search engine',
> 
> I'm not sure how to test for this.

I do not have MySQL5.7 but:

mysql> insert into search_field(name, label, type)values('name', 'label', '');
Query OK, 1 row affected, 1 warning (0.01 sec)

mysql> show warnings;
+---------+------+-------------------------------------------+
| Level   | Code | Message                                   |
+---------+------+-------------------------------------------+
| Warning | 1265 | Data truncated for column 'type' at row 1 |
+---------+------+-------------------------------------------+
1 row in set (0.00 sec)

And given https://dev.mysql.com/doc/refman/5.7/en/constraint-enum.html:
"For a column defined as ENUM('a','b','c'), values such as '', 'd', or 'ax' are invalid and are rejected."
I guess MySQL5.7 will raise an error instead of a warning.

ENUM should be avoided, but that would be for another bug report.
Comment 4 M. Tompsett 2017-01-27 17:56:51 UTC
Created attachment 59621 [details] [review]
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

The loading of file admin/searchengine/elasticsearch/mappings.yaml
specifies 'type' as empty, which fails with Mysql 5.7+ which is more
stringent.

Also, forcing an empty value into a boolean also fails.

Both issues arise when updatedatabase.pl hit 3.23.00.050.

NOTE: both issues could also be resolved by actually setting values in
      the load file. This doesn't make this solution incorrect, though.

To Test/reproduce:
-1) Happens with Mysql 5.7.4.  Maybe earlier, but certainly at
    that point. Use a Xenial kohadevbox to more easily test.
0) Find a database on 3.22 or earlier, save it.
    a) place
           sql_mode=IGNORE_SPACE,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
       into the [mysqld] section of /etc/mysql/mysql.conf.d/mysqld.cnf
    b) restart you mysql server
    c) drop the db and recreate it
    d) checkout the 3.22.x branch
    e) do a web install
    f) remove the added sql_mode
    g) restart the mysql server
1) Set your code base to master
2) run updatedatabase.pl
3) See the errors on 3.23.00.050
4) Apply the patch
5) Reload the 3.22 db.
   a) repeat steps 0(a)-0(g)
   b) don't forget the caching issues
6) succeed with updatedatabase.pl
7) drop the db and recreate it
8) run the web installer
9) notice no issues either.
10) run koha qa test tools

NOTE: This bug only solved the upgrade portion.
      I added the kohastructure.sql change as well.
      I confirmed that all the code changes were
      triggered with this test plan.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 5 Mehdi Hamidi 2017-02-21 17:40:02 UTC
Created attachment 60532 [details] [review]
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

The loading of file admin/searchengine/elasticsearch/mappings.yaml
specifies 'type' as empty, which fails with Mysql 5.7+ which is more
stringent.

Also, forcing an empty value into a boolean also fails.

Both issues arise when updatedatabase.pl hit 3.23.00.050.

NOTE: both issues could also be resolved by actually setting values in
      the load file. This doesn't make this solution incorrect, though.

To Test/reproduce:
-1) Happens with Mysql 5.7.4.  Maybe earlier, but certainly at
    that point. Use a Xenial kohadevbox to more easily test.
0) Find a database on 3.22 or earlier, save it.
    a) place
           sql_mode=IGNORE_SPACE,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
       into the [mysqld] section of /etc/mysql/mysql.conf.d/mysqld.cnf
    b) restart you mysql server
    c) drop the db and recreate it
    d) checkout the 3.22.x branch
    e) do a web install
    f) remove the added sql_mode
    g) restart the mysql server
1) Set your code base to master
2) run updatedatabase.pl
3) See the errors on 3.23.00.050
4) Apply the patch
5) Reload the 3.22 db.
   a) repeat steps 0(a)-0(g)
   b) don't forget the caching issues
6) succeed with updatedatabase.pl
7) drop the db and recreate it
8) run the web installer
9) notice no issues either.
10) run koha qa test tools

NOTE: This bug only solved the upgrade portion.
      I added the kohastructure.sql change as well.
      I confirmed that all the code changes were
      triggered with this test plan.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Mehdi Hamidi <mehdi.hamidi@inlibro.com>
Comment 6 Jonathan Druart 2017-02-22 14:35:46 UTC
Who is the original author of this patch?
The patch says "From: Your Full Name <your_email>"
Comment 7 Mehdi Hamidi 2017-02-22 14:40:33 UTC
(In reply to Jonathan Druart from comment #6)
> Who is the original author of this patch?
> The patch says "From: Your Full Name <your_email>"

The original author is : Blou <philippe.blouin@inlibro.com>
Comment 8 Jonathan Druart 2017-02-22 14:53:57 UTC
So I guess you can resubmit the patch with the correct author line.
Comment 9 Mehdi Hamidi 2017-02-22 15:04:14 UTC
Created attachment 60561 [details] [review]
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

The loading of file admin/searchengine/elasticsearch/mappings.yaml
specifies 'type' as empty, which fails with Mysql 5.7+ which is more
stringent.

Also, forcing an empty value into a boolean also fails.

Both issues arise when updatedatabase.pl hit 3.23.00.050.

NOTE: both issues could also be resolved by actually setting values in
      the load file. This doesn't make this solution incorrect, though.

To Test/reproduce:
-1) Happens with Mysql 5.7.4.  Maybe earlier, but certainly at
    that point. Use a Xenial kohadevbox to more easily test.
0) Find a database on 3.22 or earlier, save it.
    a) place
           sql_mode=IGNORE_SPACE,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
       into the [mysqld] section of /etc/mysql/mysql.conf.d/mysqld.cnf
    b) restart you mysql server
    c) drop the db and recreate it
    d) checkout the 3.22.x branch
    e) do a web install
    f) remove the added sql_mode
    g) restart the mysql server
1) Set your code base to master
2) run updatedatabase.pl
3) See the errors on 3.23.00.050
4) Apply the patch
5) Reload the 3.22 db.
   a) repeat steps 0(a)-0(g)
   b) don't forget the caching issues
6) succeed with updatedatabase.pl
7) drop the db and recreate it
8) run the web installer
9) notice no issues either.
10) run koha qa test tools

NOTE: This bug only solved the upgrade portion.
      I added the kohastructure.sql change as well.
      I confirmed that all the code changes were
      triggered with this test plan.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Mehdi Hamidi <mehdi.hamidi@inlibro.com>
Comment 10 Jonathan Druart 2017-02-24 08:09:05 UTC
Created attachment 60635 [details] [review]
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

The loading of file admin/searchengine/elasticsearch/mappings.yaml
specifies 'type' as empty, which fails with Mysql 5.7+ which is more
stringent.

Also, forcing an empty value into a boolean also fails.

Both issues arise when updatedatabase.pl hit 3.23.00.050.

NOTE: both issues could also be resolved by actually setting values in
      the load file. This doesn't make this solution incorrect, though.

To Test/reproduce:
-1) Happens with Mysql 5.7.4.  Maybe earlier, but certainly at
    that point. Use a Xenial kohadevbox to more easily test.
0) Find a database on 3.22 or earlier, save it.
    a) place
           sql_mode=IGNORE_SPACE,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
       into the [mysqld] section of /etc/mysql/mysql.conf.d/mysqld.cnf
    b) restart you mysql server
    c) drop the db and recreate it
    d) checkout the 3.22.x branch
    e) do a web install
    f) remove the added sql_mode
    g) restart the mysql server
1) Set your code base to master
2) run updatedatabase.pl
3) See the errors on 3.23.00.050
4) Apply the patch
5) Reload the 3.22 db.
   a) repeat steps 0(a)-0(g)
   b) don't forget the caching issues
6) succeed with updatedatabase.pl
7) drop the db and recreate it
8) run the web installer
9) notice no issues either.
10) run koha qa test tools

NOTE: This bug only solved the upgrade portion.
      I added the kohastructure.sql change as well.
      I confirmed that all the code changes were
      triggered with this test plan.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Mehdi Hamidi <mehdi.hamidi@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2017-02-24 08:09:15 UTC
Created attachment 60636 [details] [review]
Bug 17260: Update existing installs

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Kyle M Hall 2017-03-03 16:47:38 UTC
Pushed to master for 17.05, thanks Phillipe, Jonathan!
Comment 13 Katrin Fischer 2017-03-12 22:05:19 UTC
These patches have been pushed to 16.11.x and will be in 16.11.05.
Comment 14 Katrin Fischer 2017-03-12 22:13:09 UTC
These patches have been pushed to 16.11.x and will be in 16.11.05.
Comment 15 Mason James 2017-04-03 14:42:46 UTC
Pushed to 16.05.x, for 16.05.11 release