[Bug 7872] New: C4::Items GetItemsInfo marc_subfield_structure SQL queries fetch all results and use first row
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Priority: P5 - low Change sponsored?: --- Bug ID: 7872 Assignee: gmcharlt@gmail.com Summary: C4::Items GetItemsInfo marc_subfield_structure SQL queries fetch all results and use first row QA Contact: koha.sekjal@gmail.com Severity: enhancement Classification: Unclassified OS: All Reporter: dpavlin@rot13.org Hardware: All Status: NEW Version: master Component: MARC Authority data support Product: Koha There are two queries without any WHERE which return more than one row, but use just first one (since they don't have order which one will be returned is database dependent). SELECT authorised_value FROM marc_subfield_structure WHERE kohafield="items.notforloan" SELECT authorised_value FROM marc_subfield_structure WHERE kohafield="items.restricted" -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 --- Comment #1 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 8774 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8774&action=edit Bug 7872 - C4::Items::GetItemsInfo marc_subfield_structure SQL queries fetch all results and use first row. Adding frameworkcode to SQL queries will help returning just one (correct) row, improve performance and allow caching of values in future. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Blocks| |7172 Assignee|gmcharlt@gmail.com |dpavlin@rot13.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |7119 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alex.arnaud@biblibre.com --- Comment #2 from Alex Arnaud <alex.arnaud@biblibre.com> --- Interesting patch, not only for performances but also functionally. let's assume we have several bibliographic frameworks using different authorized value categories for notforloan field (A librarian told me that functionally it doesn't make sense. But technically it's possible). In this case, some items get the notforloan description from an other authorized value than its framework is configured for. I think it depends on the order rows are returned. So, changing importance to normal bug. And signing off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8774|0 |1 is obsolete| | --- Comment #3 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 8904 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8904&action=edit [Signed-off] Bug 7872 - C4::Items::GetItemsInfo marc_subfield_structure SQL queries fetch all results and use first row. Adding frameworkcode to SQL queries will help returning just one (correct) row, improve performance and allow caching of values in future. Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA Comment: Hi Dobrica, Why don't you replace these queries with a call to the C4::Koha::GetAuthValCode function (it does the same query) ? However, isn't it better to use the LIMIT 1 clause if we just want one result ? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #5 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Jonathan, great point about C4::Koha::GetAuthValCode. I will rewrite patch and as side-benefit we can memoize it later for bug 7177. LIMIT 1 is not needed since adding framework to WHERE always returns single row and it's more correct. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to comment #5)
LIMIT 1 is not needed since adding framework to WHERE always returns single row and it's more correct.
Why more correct ? I don't understand. For me, LIMIT 1 should be used if we want only a unique row. I think it can increase performance (I agree with you, it is negligible). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 --- Comment #7 from Alex Arnaud <alex.arnaud@biblibre.com> --- Jonathan, we need to get the right row, not just 1 row. The right one depends on the framework code of the record (see comment 2). But C4::Koha::GetAuthValCode seems to be the best way to do that. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|C4::Items GetItemsInfo |C4::Items::GetItemsInfo |marc_subfield_structure SQL |should use C4::Koha instead |queries fetch all results |of SQL queries directly |and use first row | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8904|0 |1 is obsolete| | --- Comment #8 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 8917 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8917&action=edit Bug 7872 - C4::Items::GetItemsInfo should use C4::Koha instead of SQL queries directly This simplifies code, and has nice side-effect that memoize of C4::Koha functions will be more effective. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |7177 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8917|0 |1 is obsolete| | --- Comment #9 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 8923 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8923&action=edit Bug 7872 - C4::Items::GetItemsInfo should use C4::Koha instead of SQL queries directly This simplifies code, and has nice side-effect that memoize of C4::Koha functions will be more effective. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|C4::Items::GetItemsInfo |C4::Items should use |should use C4::Koha instead |C4::Koha methods instead of |of SQL queries directly |duplicating SQL queries -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #10 from Chris Cormack <chris@bigballofwax.co.nz> --- (In reply to comment #6)
(In reply to comment #5)
LIMIT 1 is not needed since adding framework to WHERE always returns single row and it's more correct.
Why more correct ? I don't understand. For me, LIMIT 1 should be used if we want only a unique row. I think it can increase performance (I agree with you, it is negligible).
Actually a where on an indexed column is a lot faster than using limit, explain is a good tool for looking at the query plan to see how mysql is optimising the query. There is a whole project of sql optimisation we could do by checking some of our more used queries. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8923|0 |1 is obsolete| | --- Comment #11 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 9993 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=9993&action=edit Bug 7872 - C4::Items::GetItemsInfo should use C4::Koha instead of SQL queries directly This simplifies code, and has nice side-effect that memoize of C4::Koha functions will be more effective. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |paul.poulain@biblibre.com --- Comment #12 from Paul Poulain <paul.poulain@biblibre.com> --- QA comment: this patch replaces 30 lines by 3, using an existing function to avoid duplicaing code dobrica++ ! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 --- Comment #13 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #10)
(In reply to comment #6)
(In reply to comment #5)
LIMIT 1 is not needed since adding framework to WHERE always returns single row and it's more correct.
Why more correct ? I don't understand. For me, LIMIT 1 should be used if we want only a unique row. I think it can increase performance (I agree with you, it is negligible).
Actually a where on an indexed column is a lot faster than using limit, explain is a good tool for looking at the query plan to see how mysql is optimising the query. There is a whole project of sql optimisation we could do by checking some of our more used queries.
Site comment: investigating DBI, we've found that all handlers are automatically closed when a query has returned all results. If plan not to fetch all results, then you must explicitely close $sth, or you may have some useless memory consumption -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7872 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master Version|master |rel_3_10 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org