acqui/ordered.pl: GROUP BY aqorders.ordernumber acqui/spent.pl: GROUP BY aqorders.ordernumber 'koha_kohadev.aqorders.biblionumber' isn't in GROUP BY
Created attachment 80943 [details] [review] Bug 21622: Remove incorrect GROUP BY clauses in acqui script acqui/ordered.pl: GROUP BY aqorders.ordernumber acqui/spent.pl: GROUP BY aqorders.ordernumbe 'koha_kohadev.aqorders.biblionumber' isn't in GROUP BY Test plan: - Create orders for different basket and using different funds - Receive some of them - Hit the ordered and spent pages (from the acqui home page) => The tables must contain the same data with and without this patch
*** Bug 21718 has been marked as a duplicate of this bug. ***
Created attachment 81596 [details] [review] Bug 21622: Remove incorrect GROUP BY clauses in acqui script acqui/ordered.pl: GROUP BY aqorders.ordernumber acqui/spent.pl: GROUP BY aqorders.ordernumbe 'koha_kohadev.aqorders.biblionumber' isn't in GROUP BY Test plan: - Create orders for different basket and using different funds - Receive some of them - Hit the ordered and spent pages (from the acqui home page) => The tables must contain the same data with and without this patch Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
The SQL queries have a JOIN clause with aqorders_items. Without the GROUP BY it can return multiple rows for the same order. It shouldn't be removed. On master, ordered.pl displays a table of 15 rows With the patch applied, it displays a table of 22 rows Same thing on spent.pl with different numbers. Failed QA
Thx Julian! It didn't happen on mine, probably not enough data!
Created attachment 81724 [details] [review] Bug 21622: Adjust incorrect GROUP BY clauses in acqui script acqui/ordered.pl: GROUP BY aqorders.ordernumber acqui/spent.pl: GROUP BY aqorders.ordernumbe 'koha_kohadev.aqorders.biblionumber' isn't in GROUP BY Test plan: - Create orders for different basket and using different funds - Receive some of them - Hit the ordered and spent pages (from the acqui home page) => The tables must contain the same data with and without this patch
Created attachment 81725 [details] [review] Bug 21622: Adjust incorrect GROUP BY clauses in acqui script acqui/ordered.pl: GROUP BY aqorders.ordernumber acqui/spent.pl: GROUP BY aqorders.ordernumbe 'koha_kohadev.aqorders.biblionumber' isn't in GROUP BY Test plan: - Create orders for different basket and using different funds - Receive some of them - Hit the ordered and spent pages (from the acqui home page) => The tables must contain the same data with and without this patch
Created attachment 84101 [details] [review] Bug 21622: Adjust incorrect GROUP BY clauses in acqui script acqui/ordered.pl: GROUP BY aqorders.ordernumber acqui/spent.pl: GROUP BY aqorders.ordernumbe 'koha_kohadev.aqorders.biblionumber' isn't in GROUP BY Test plan: - Create orders for different basket and using different funds - Receive some of them - Hit the ordered and spent pages (from the acqui home page) => The tables must contain the same data with and without this patch Signed-off-by: Jasmine Amohia <jasmineamohia.student@wegc.school.nz>
Created attachment 84679 [details] [review] Bug 21622: Adjust incorrect GROUP BY clauses in acqui script acqui/ordered.pl: GROUP BY aqorders.ordernumber acqui/spent.pl: GROUP BY aqorders.ordernumbe 'koha_kohadev.aqorders.biblionumber' isn't in GROUP BY Test plan: - Create orders for different basket and using different funds - Receive some of them - Hit the ordered and spent pages (from the acqui home page) => The tables must contain the same data with and without this patch Signed-off-by: Jasmine Amohia <jasmineamohia.student@wegc.school.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I missed this one and filed 22260 I am wondering if one situation is preferable to the other (subquery vs adding all fields to group by) Bug 20182 and 21723 imply there may be some issues here - can we get some opinions here on the best way to fix these situations?
(In reply to Nick Clemens from comment #10) > I missed this one and filed 22260 > > I am wondering if one situation is preferable to the other (subquery vs > adding all fields to group by) > > Bug 20182 and 21723 imply there may be some issues here - can we get some > opinions here on the best way to fix these situations? Could you benchmark the 2 queries on a big production DB?
*** Bug 22260 has been marked as a duplicate of this bug. ***
(In reply to Jonathan Druart from comment #11) > (In reply to Nick Clemens from comment #10) > > I missed this one and filed 22260 > > > > I am wondering if one situation is preferable to the other (subquery vs > > adding all fields to group by) > > > > Bug 20182 and 21723 imply there may be some issues here - can we get some > > opinions here on the best way to fix these situations? > > Could you benchmark the 2 queries on a big production DB? ~.1 seconds for this solution vs ~4 seconds for the other Deprecated in favor of this one, we should invesitgate 21723 on its own I think
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.04
this one fails qa tool in 18.05.x AND i cannot successfully run through the test plan. not backporting to 18.05.x. let me know if it is needed
Created attachment 86973 [details] [review] Bug 21622: (RM follow-up) adjust for changes from bug 21282
Created attachment 86974 [details] [review] Bug 21622: (RM follow-up) adjust for changes from bug 21282
(In reply to Nick Clemens from comment #18) > Created attachment 86974 [details] [review] [review] > Bug 21622: (RM follow-up) adjust for changes from bug 21282 Followup pushed to master
Followup pushed to 18.11.x series.