Bug 20182 - Remove group by clause in search_patrons_to_anonymise
Summary: Remove group by clause in search_patrons_to_anonymise
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Julian Maurice
URL:
Keywords:
Depends on:
Blocks: 17258 21723
  Show dependency treegraph
 
Reported: 2018-02-12 14:29 UTC by Julian Maurice
Modified: 2019-10-14 19:57 UTC (History)
0 users

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20182: [sql_modes] Remove group by clause in search_patrons_to_anonymise (1.01 KB, patch)
2018-02-12 14:30 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20182: [sql_modes] Remove group by clause in search_patrons_to_anonymise (1.07 KB, patch)
2018-02-15 10:19 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.