Bug 28523 - Patrons with the most checkouts (bor_issues_top.pl) is failing with MySQL 8
Summary: Patrons with the most checkouts (bor_issues_top.pl) is failing with MySQL 8
Status: Pushed to oldoldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 23996
  Show dependency treegraph
 
Reported: 2021-06-07 21:03 UTC by Humberto Chula
Modified: 2021-09-11 20:46 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.02,20.11.08,20.05.14,19.11.20


Attachments
Bug 28523: Escape 'rank' in bor_issues_top.pl (1.76 KB, patch)
2021-06-16 12:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28523: Escape 'rank' in bor_issues_top.pl (1.82 KB, patch)
2021-06-24 16:44 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28523: Escape 'rank' in bor_issues_top.pl (1.91 KB, patch)
2021-06-25 07:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Humberto Chula 2021-06-07 21:03:17 UTC
CGI::Compile::ROOT::usr_share_koha_intranet_cgi_2dbin_reports_bor_issues_top_2epl::calculate(): DBI Exception: DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'RANK, borrowers.borrowernumber AS ID FROM `old_issues`
                  LEFT JO' at line 1 [for Statement "SELECT  CONCAT_WS('', borrowers.surname , ",\t", borrowers.firstname),  COUNT(*) AS RANK, borrowers.borrowernumber AS ID FROM `old_issues`
                  LEFT JOIN  borrowers  USING(borrowernumber)
                  LEFT JOIN    items    USING(itemnumber)
                  LEFT JOIN biblioitems USING(biblioitemnumber)
                  WHERE old_issues.borrowernumber IS NOT NULL
                   AND old_issues.issuedate > '2021-06-01'  AND old_issues.issuedate < '2021-06-07'  GROUP BY borrowers.borrowernumber ORDER BY RANK DESC LIMIT 5"] at /usr/share/koha/intranet/cgi-bin/reports/bor_issues_top.pl line 70


Rank is a MySQL reserved word defined in MySQL version 8.0.2

https://dev.mysql.com/doc/refman/8.0/en/keywords.html#keywords-8-0-detailed-R
Comment 1 Jonathan Druart 2021-06-16 12:56:18 UTC
Created attachment 122037 [details] [review]
Bug 28523: Escape 'rank' in bor_issues_top.pl

It's a MySQL 8 keyword

Test plan:
Turn off strict_sql_modes (there are other problems in this script)
Hit Home Reports > Patrons with the most checkouts
Submit the form

Without this patch you got:
    You have an error in your SQL syntax; check the manual that corresponds to your MySQL s
erver version for the right syntax to use near 'RANK, borrowers.borrowernumber AS ID FROM `old_issues`

With this patch applied you see the report result view
Comment 2 David Nind 2021-06-18 18:39:14 UTC
I attempted to test this.

With MySQL 8 I was able to get this to work following the test plan (starting KTD with ku=my8).

However, if I change back to MariaDB (starting KTD with ku) I get a different error message:

CGI::Compile::ROOT::kohadevbox_koha_reports_bor_issues_top_2epl::calculate(): DBI Exception: DBD::mysql::st execute failed: 'koha_kohadev.borrowers.surname' isn't in GROUP BY [for Statement "SELECT  CONCAT_WS('', borrowers.surname , ",\t", borrowers.firstname),  COUNT(*) AS `RANK`, borrowers.borrowernumber AS ID FROM `old_issues`
                  LEFT JOIN  borrowers  USING(borrowernumber)
                  LEFT JOIN    items    USING(itemnumber)
                  LEFT JOIN biblioitems USING(biblioitemnumber)
                  WHERE old_issues.borrowernumber IS NOT NULL
                   AND old_issues.issuedate > '2021-06-01'  AND old_issues.issuedate < '2021-06-30'  AND old_issues.returndate > '2021-06-01'  AND old_issues.returndate < '2021-06-30'  GROUP BY borrowers.borrowernumber ORDER BY `RANK` DESC LIMIT 5"] at /kohadevbox/koha/reports/bor_issues_top.pl line 70
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77


Testing notes (koha-testing-docker):
- Start KTD with MySQL 8 (rather than MariaDB): ku-my8
- Turn off strict mode (https://www.linode.com/community/questions/17070/how-can-i-disable-mysql-strict-mode):
  . access database container: docker exec -it koha_db_1 bash
  . mysql -uroot -ppassword
  . SET GLOBAL sql_mode = 'NO_ENGINE_SUBSTITUTION';
  . SELECT @@GLOBAL.sql_mode;
Comment 3 Jonathan Druart 2021-06-21 12:19:29 UTC
Hi David, 

The "isn't in GROUP BY" error is happening when strict modes are set.

To turn them off you need to edit $KOHA_CONF and set the value for <strict_sql_modes> to 0.

Setting them globally is not enough as we are setting them when we initiate the connection to the DB:

https://git.koha-community.org/Koha-community/Koha/src/commit/d3ab8dbeec188ad21c468d82ba8692e89df77a59/Koha/Database.pm#L77
Comment 4 David Nind 2021-06-24 14:06:52 UTC
Thanks Jonathan!

I can now get this to work with MySQL 8 (ku-my8).

With the patch applied in master with MariaDB (ku), I need to turn off strict mode to get this to work. Is that what is expected, or is another bug going to fix this?

David
Comment 5 Owen Leonard 2021-06-24 16:44:39 UTC
Created attachment 122395 [details] [review]
Bug 28523: Escape 'rank' in bor_issues_top.pl

It's a MySQL 8 keyword

Test plan:
Turn off strict_sql_modes (there are other problems in this script)
Hit Home Reports > Patrons with the most checkouts
Submit the form

Without this patch you got:
    You have an error in your SQL syntax; check the manual that
    corresponds to your MySQL server version for the right syntax to
    use near 'RANK, borrowers.borrowernumber AS ID FROM `old_issues`

With this patch applied you see the report result view

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Marcel de Rooy 2021-06-25 07:15:35 UTC
Created attachment 122417 [details] [review]
Bug 28523: Escape 'rank' in bor_issues_top.pl

It's a MySQL 8 keyword

Test plan:
Turn off strict_sql_modes (there are other problems in this script)
Hit Home Reports > Patrons with the most checkouts
Submit the form

Without this patch you got:
    You have an error in your SQL syntax; check the manual that
    corresponds to your MySQL server version for the right syntax to
    use near 'RANK, borrowers.borrowernumber AS ID FROM `old_issues`

With this patch applied you see the report result view

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2021-07-06 09:06:03 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 8 Kyle M Hall 2021-07-09 16:30:12 UTC
Pushed to 21.05.x for 21.05.02
Comment 9 Victor Grousset/tuxayo 2021-07-22 14:38:32 UTC
Hi, Fridolin impostor here.
Backported: Pushed to 20.11.x branch for 20.11.08
Comment 10 Victor Grousset/tuxayo 2021-07-22 19:31:13 UTC
Backported: Pushed to 20.05.x branch for 20.05.14
Comment 11 wainuiwitikapark 2021-07-25 04:42:10 UTC
Backported to 19.11.x for 19.11.20
Comment 12 David Nind 2021-09-10 21:06:44 UTC
Currently getting this error when running Patrons with the most checkouts report (using KTD started with ku and on master) - is the this the same issue or something else?

CGI::Compile::ROOT::kohadevbox_koha_reports_bor_issues_top_2epl::calculate(): DBI Exception: DBD::mysql::st execute failed: 'koha_kohadev.borrowers.surname' isn't in GROUP BY at /kohadevbox/koha/reports/bor_issues_top.pl line 63
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

    in DBIx::Class::Exception::throw at /usr/share/perl5/DBIx/Class/Exception.pm line 77

       74:     my $self = { msg => $msg };
       75:     bless $self => $class;
       76: 
       77:     die $self;
       78: }
       79: 
       80: =head2 rethrow

    Show function arguments
    in DBIx::Class::Schema::throw_exception at /usr/share/perl5/DBIx/Class/Schema.pm line 1118

     1115:     };
     1116:   }
     1117: 
     1118:   DBIx::Class::Exception->throw($args[0], $self->stacktrace);
     1119: }
     1120: 
     1121: =head2 deploy

    Show function arguments
    in DBIx::Class::Storage::throw_exception at /usr/share/perl5/DBIx/Class/Storage.pm line 113

      110:   my $self = shift;
      111: 
      112:   if (ref $self and $self->schema) {
      113:     $self->schema->throw_exception(@_);
      114:   }
      115:   else {
      116:     DBIx::Class::Exception->throw(@_);
Comment 13 Michael Hafen 2021-09-10 22:50:00 UTC
That is something else. The first line of the test plan states that there are other problems with the script, this is the other problem.  In SQL Strict mode the query failed the ONLY_FULL_GROUP_BY check.

I recently ran into that problem (and created a patch) on another report.

(In reply to David Nind from comment #12)
> Currently getting this error when running Patrons with the most checkouts
> report (using KTD started with ku and on master) - is the this the same
> issue or something else?
> 
> CGI::Compile::ROOT::kohadevbox_koha_reports_bor_issues_top_2epl::calculate():
> DBI Exception: DBD::mysql::st execute failed:
> 'koha_kohadev.borrowers.surname' isn't in GROUP BY at
> /kohadevbox/koha/reports/bor_issues_top.pl line 63
>  at /usr/share/perl5/DBIx/Class/Exception.pm line 77
> 
>     in DBIx::Class::Exception::throw at
> /usr/share/perl5/DBIx/Class/Exception.pm line 77
> 
>        74:     my $self = { msg => $msg };
>        75:     bless $self => $class;
>        76: 
>        77:     die $self;
>        78: }
>        79: 
>        80: =head2 rethrow
> 
>     Show function arguments
>     in DBIx::Class::Schema::throw_exception at
> /usr/share/perl5/DBIx/Class/Schema.pm line 1118
> 
>      1115:     };
>      1116:   }
>      1117: 
>      1118:   DBIx::Class::Exception->throw($args[0], $self->stacktrace);
>      1119: }
>      1120: 
>      1121: =head2 deploy
> 
>     Show function arguments
>     in DBIx::Class::Storage::throw_exception at
> /usr/share/perl5/DBIx/Class/Storage.pm line 113
> 
>       110:   my $self = shift;
>       111: 
>       112:   if (ref $self and $self->schema) {
>       113:     $self->schema->throw_exception(@_);
>       114:   }
>       115:   else {
>       116:     DBIx::Class::Exception->throw(@_);
Comment 14 David Nind 2021-09-11 20:46:41 UTC
Thanks Michael!