Description
Jonathan Druart
2018-10-19 15:18:14 UTC
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. |