Bug 20182

Summary: Remove group by clause in search_patrons_to_anonymise
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: Test SuiteAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Julian Maurice <julian.maurice>
Severity: normal    
Priority: P5 - low    
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20144
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 17258, 21723    
Attachments: Bug 20182: [sql_modes] Remove group by clause in search_patrons_to_anonymise
Bug 20182: [sql_modes] Remove group by clause in search_patrons_to_anonymise

Description Julian Maurice 2018-02-12 14:29:38 UTC
This bug is a continuation of bug 20144
Comment 1 Julian Maurice 2018-02-12 14:30:40 UTC
Created attachment 71475 [details] [review]
Bug 20182: [sql_modes] Remove group by clause in search_patrons_to_anonymise

Not trivial, more attention needed on this one!

Fix for:
'koha_kohadev.me.cardnumber' isn't in GROUP BY

Looks like the query is complexified, perf tests needed, see bug 19599.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 2 Julian Maurice 2018-02-12 14:35:52 UTC
perl -MKoha::Patrons -E 'my $patrons = Koha::Patrons->search_patrons_to_anonymise; $patrons->next'

generated the following SQL query:

SELECT `me`.`borrowernumber`, `me`.`cardnumber`, `me`.`surname`, `me`.`firstname`, `me`.`title`, `me`.`othernames`, `me`.`initials`, `me`.`streetnumber`, `me`.`streettype`, `me`.`address`, `me`.`address2`, `me`.`city`, `me`.`state`, `me`.`zipcode`, `me`.`country`, `me`.`email`, `me`.`phone`, `me`.`mobile`, `me`.`fax`, `me`.`emailpro`, `me`.`phonepro`, `me`.`B_streetnumber`, `me`.`B_streettype`, `me`.`B_address`, `me`.`B_address2`, `me`.`B_city`, `me`.`B_state`, `me`.`B_zipcode`, `me`.`B_country`, `me`.`B_email`, `me`.`B_phone`, `me`.`dateofbirth`, `me`.`branchcode`, `me`.`categorycode`, `me`.`dateenrolled`, `me`.`dateexpiry`, `me`.`date_renewed`, `me`.`gonenoaddress`, `me`.`lost`, `me`.`debarred`, `me`.`debarredcomment`, `me`.`contactname`, `me`.`contactfirstname`, `me`.`contacttitle`, `me`.`guarantorid`, `me`.`borrowernotes`, `me`.`relationship`, `me`.`sex`, `me`.`password`, `me`.`flags`, `me`.`userid`, `me`.`opacnote`, `me`.`contactnote`, `me`.`sort1`, `me`.`sort2`, `me`.`altcontactfirstname`, `me`.`altcontactsurname`, `me`.`altcontactaddress1`, `me`.`altcontactaddress2`, `me`.`altcontactaddress3`, `me`.`altcontactstate`, `me`.`altcontactzipcode`, `me`.`altcontactcountry`, `me`.`altcontactphone`, `me`.`smsalertnumber`, `me`.`sms_provider_id`, `me`.`privacy`, `me`.`privacy_guarantor_checkouts`, `me`.`checkprevcheckout`, `me`.`updated_on`, `me`.`lastseen`, `me`.`lang`, `me`.`login_attempts`, `me`.`overdrive_auth_token` FROM `borrowers` `me` LEFT JOIN `old_issues` `old_issues` ON `old_issues`.`borrowernumber` = `me`.`borrowernumber` WHERE ( ( `old_issues`.`borrowernumber` IS NOT NULL AND `privacy` <> '0' AND `returndate` < '2018-02-12 15:32:29' ) ) GROUP BY `me`.`borrowernumber`, `me`.`cardnumber`, `me`.`surname`, `me`.`firstname`, `me`.`title`, `me`.`othernames`, `me`.`initials`, `me`.`streetnumber`, `me`.`streettype`, `me`.`address`, `me`.`address2`, `me`.`city`, `me`.`state`, `me`.`zipcode`, `me`.`country`, `me`.`email`, `me`.`phone`, `me`.`mobile`, `me`.`fax`, `me`.`emailpro`, `me`.`phonepro`, `me`.`B_streetnumber`, `me`.`B_streettype`, `me`.`B_address`, `me`.`B_address2`, `me`.`B_city`, `me`.`B_state`, `me`.`B_zipcode`, `me`.`B_country`, `me`.`B_email`, `me`.`B_phone`, `me`.`dateofbirth`, `me`.`branchcode`, `me`.`categorycode`, `me`.`dateenrolled`, `me`.`dateexpiry`, `me`.`date_renewed`, `me`.`gonenoaddress`, `me`.`lost`, `me`.`debarred`, `me`.`debarredcomment`, `me`.`contactname`, `me`.`contactfirstname`, `me`.`contacttitle`, `me`.`guarantorid`, `me`.`borrowernotes`, `me`.`relationship`, `me`.`sex`, `me`.`password`, `me`.`flags`, `me`.`userid`, `me`.`opacnote`, `me`.`contactnote`, `me`.`sort1`, `me`.`sort2`, `me`.`altcontactfirstname`, `me`.`altcontactsurname`, `me`.`altcontactaddress1`, `me`.`altcontactaddress2`, `me`.`altcontactaddress3`, `me`.`altcontactstate`, `me`.`altcontactzipcode`, `me`.`altcontactcountry`, `me`.`altcontactphone`, `me`.`smsalertnumber`, `me`.`sms_provider_id`, `me`.`privacy`, `me`.`privacy_guarantor_checkouts`, `me`.`checkprevcheckout`, `me`.`updated_on`, `me`.`lastseen`, `me`.`lang`, `me`.`login_attempts`, `me`.`overdrive_auth_token`

So it basically just added a GROUP BY with all columns. Can it affect performances ?
Comment 3 Jonathan Druart 2018-02-14 17:48:58 UTC
(In reply to Julian Maurice from comment #2)
> So it basically just added a GROUP BY with all columns. Can it affect
> performances ?

EXPLAIN tells me it should not.
Comment 4 Julian Maurice 2018-02-15 10:19:16 UTC
Created attachment 71642 [details] [review]
Bug 20182: [sql_modes] Remove group by clause in search_patrons_to_anonymise

Not trivial, more attention needed on this one!

Fix for:
'koha_kohadev.me.cardnumber' isn't in GROUP BY

Looks like the query is complexified, perf tests needed, see bug 19599.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 5 Jonathan Druart 2018-02-16 17:07:06 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 6 Jonathan Druart 2018-11-01 16:12:18 UTC
(In reply to Jonathan Druart from comment #3)
> (In reply to Julian Maurice from comment #2)
> > So it basically just added a GROUP BY with all columns. Can it affect
> > performances ?
> 
> EXPLAIN tells me it should not.

Looking at bug 21723, it may be wrong.