[Bug 8329] New: GetLostItems in C4::Items.pm has a SELECT *
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 Priority: P5 - low Change sponsored?: --- Bug ID: 8329 Assignee: gmcharlt@gmail.com Summary: GetLostItems in C4::Items.pm has a SELECT * Severity: major Classification: Unclassified OS: All Reporter: mtompset@hotmail.com Hardware: All Status: NEW Version: unspecified Component: Reports Product: Koha my $query = " SELECT * FROM items LEFT JOIN biblio ON (items.biblionumber = biblio.biblionumber) LEFT JOIN biblioitems ON (items.biblionumber = biblioitems.biblionumber) LEFT JOIN authorised_values ON (items.itemlost = authorised_values.authorised_value) WHERE authorised_values.category = 'LOST' AND itemlost IS NOT NULL AND itemlost <> 0 "; We're using 3.6.3, and I comfirmed this is in 3.6.6. This problem would only be obvious if there are many lost items. It is a hellish query as we have 39,000+ "lost" items. We're in the midst of doing data entry/cleanup on a new system. my $query = " SELECT title, author, lib, itemlost, authorised_value, barcode, datelastseen, price, replacementprice, homebranch, itype, itemtype, holdingbranch, location, itemnotes, items.biblionumber as biblionumber FROM items LEFT JOIN biblio ON (items.biblionumber = biblio.biblionumber) LEFT JOIN biblioitems ON (items.biblionumber = biblioitems.biblionumber) LEFT JOIN authorised_values ON (items.itemlost = authorised_values.authorised_value) WHERE authorised_values.category = 'LOST' AND itemlost IS NOT NULL AND itemlost <> 0 "; This is functional, though it can probably be trimmed. The SELECT * includes XML and MARC fields which are sizable and eat system memory! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 10565 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10565&action=edit Reduced memory requirements of a massive select statement SELECT * joining items, bibioitems, and biblio is bad. The MARC and XML fields will eat memory very quickly. Our server has ridiculously low memory and lots of items marked as lost. We were getting Error 500 pages as output as a result. Since the GetLostItems function is only used by the Items Lost report, I looked at the template in reports/itemslost.tt to get an idea of what might better be suited in the SELECT statement. It was horribly slow, but finally worked. I need to get more than 512MB for our server. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10565|0 |1 is obsolete| | --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 11102 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11102&action=edit Bug 8329 - GetLostItems in C4::Items.pm has a SELECT * Provided a smaller list based on reports/itemlost.tt Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.poulain@biblibre.com --- Comment #3 from Paul Poulain <paul.poulain@biblibre.com> --- QA comment: this patch looks good, but a small question: why not SELECT biblio,author,...,items.* (all items fields) ? would that solve your performance problem ? if yest, I think it's better, because it will be more evolutive in case one want to introduce a new column in the report. But I won't fail QA for that, if you say enumerating fields is needed, I'll push your patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |mtj@kohaaloha.com --- Comment #4 from Mason James <mtj@kohaaloha.com> --- (In reply to comment #3)
QA comment: this patch looks good, but a small question: why not SELECT biblio,author,...,items.* (all items fields) ? would that solve your performance problem ? if yest, I think it's better, because it will be more evolutive in case one want to introduce a new column in the report.
But I won't fail QA for that, if you say enumerating fields is needed, I'll push your patch
ok Paul, all QA tests pass, patch looks good - passing QA :) mason@xen1:~/git/head$ koha-qa.pl * d4f15be Bug 8329 - GetLostItems in C4::Items.pm has a SELECT * Provided a smaller list based on reports/itemlost.tt C4/Items.pm - perlcritic-progressive tests... OK - perl -c syntax tests... OK - xt/tt_valid.t tests... OK - xt/author/valid-template.t tests... OK -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #5 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.8.x will be in 3.8.4. Re the select *, I have definitely come around to Andy Lesters' school of thought. Select * is a bug waiting to happen. http://petdance.com/2012/07/select-is-a-bug-waiting-to-happen/ -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master Version|unspecified |rel_3_8 --- Comment #6 from Paul Poulain <paul.poulain@biblibre.com> --- Patch pushed. I've only 79 lost items in my setup, the perf improvement is not noticeable. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 --- Comment #7 from Paul Poulain <paul.poulain@biblibre.com> --- additionnal QA comment: 15:34 ~/koha.dev/koha-community (new/bug_8329 $%)$ grep -R GetLostItems * C4/Items.pm: GetLostItems C4/Items.pm:=head2 GetLostItems C4/Items.pm: $items = GetLostItems( $where, $orderby ); C4/Items.pm: my $items = GetLostItems( $where, "homebranch" ); C4/Items.pm:sub GetLostItems { reports/itemslost.pl: my $items = GetLostItems( \%where, $orderbyfilter ); t/db_dependent/lib/KohaTest/Items.pm: GetLostItems => this sub is used only in reports/itemslost.pl, nothing more to test -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8329 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org