bug 23624 simplified how we get the count of rows in a report, however, it breaks under certain circumstances. A report like: SELECT * FROM issues JOIN borrowers USING (borrowernumber) will have two borrowernumber columns - SQL will give us there rsults, but if we try to wrap them in a SELECT COUNT(*) FROM (report) it throws a duplicated column error. I don't know an easy fix except to state that one must name the columns they want to avoid duplication
It seems that we should revert commit bca4453c50d8b5b1d3c7029ded2b522a9a2bf868 Bug 23624: (QA follow-up) Optimize even more From https://bugs.mysql.com/bug.php?id=6709: """ This is in agreement with the SQL Standard, which says: [...] 11) No <column name> shall be specified more than once in a <derived column> list. """
This is horrible.. I really don't like the idea of reverting that followup as it fixes an important performance hole. Feels like MySQL is getting it wrong to me.. grr Contemplating if we have any other options.
I think if we can make a sensible error back to the user about deduplicating the names we would be okay. Ideally we would also make basic pagination work, just assumign 10 pages, and again having a friendly error if they page out of bounds
Created attachment 95479 [details] [review] Bug 23982: (bug 23624 follow-up) Handle SQL error caused by derived table Caused by commit bca4453c50d8b5b1d3c7029ded2b522a9a2bf868 Bug 23624: (QA follow-up) Optimize even more A report like: SELECT * FROM issues JOIN borrowers USING (borrowernumber) will have two borrowernumber columns - SQL will give us there rsults, but if we try to wrap them in a SELECT COUNT(*) FROM (report) it throws a duplicated column error. This patch suggests to execute the query the old way if the derived table optimization failed.
This impacts stable releases and will cause reports to fail. I think it's a critical one.
Created attachment 95545 [details] [review] Bug 23982: (bug 23624 follow-up) Handle SQL error caused by derived table Caused by commit bca4453c50d8b5b1d3c7029ded2b522a9a2bf868 Bug 23624: (QA follow-up) Optimize even more A report like: SELECT * FROM issues JOIN borrowers USING (borrowernumber) will have two borrowernumber columns - SQL will give us there rsults, but if we try to wrap them in a SELECT COUNT(*) FROM (report) it throws a duplicated column error. This patch suggests to execute the query the old way if the derived table optimization failed. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks for jumping on this one Jonathan, looks reasonable and in a quick test it works for me.. Signing off.
Created attachment 95548 [details] [review] Bug 23982: (bug 23624 follow-up) Handle SQL error caused by derived table Caused by commit bca4453c50d8b5b1d3c7029ded2b522a9a2bf868 Bug 23624: (QA follow-up) Optimize even more A report like: SELECT * FROM issues JOIN borrowers USING (borrowernumber) will have two borrowernumber columns - SQL will give us there rsults, but if we try to wrap them in a SELECT COUNT(*) FROM (report) it throws a duplicated column error. This patch suggests to execute the query the old way if the derived table optimization failed. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Nice work! Pushed to master for 19.11.00
Created attachment 95593 [details] [review] Bug 23982: Fix the failing test # Failed test 'nb_rows returns 0 on bad queries' # at t/db_dependent/Reports/Guided.t line 441. # got: undef # expected: '0' Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test Fix pushed to master