https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43102 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=43101 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43102 --- Comment #1 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 202108 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202108&action=edit Bug 43102: Koha::Items filters inline every itemnumber of the set as an OR chain filter_by_checked_out, filter_by_in_transit, filter_by_has_holds and filter_by_has_recalls already restrict the final search with an IN subquery, but the subquery itself is built by fetching every itemnumber of the set into memory and passing the list as an arrayref that SQL::Abstract renders as one OR clause per itemnumber. For a records with large numbers of items this can produce very large queries. This patch passes the itemnumbers as a subquery so the database resolves the set itself. Existing unit tests should prove there is no change in behavior. Test Plan: 1) Apply this patch 2) prove t/db_dependent/Koha/Items.t 3) Note all tests pass! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43102 --- Comment #3 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 202110 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202110&action=edit Bug 43102: Add POD to _extract_custom_attrs and _process_params Patch from commit fdbbad7 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43102 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43102 --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 202112 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202112&action=edit Bug 43102: Koha::Items filters inline every itemnumber of the set as an OR chain filter_by_checked_out, filter_by_in_transit, filter_by_has_holds and filter_by_has_recalls already restrict the final search with an IN subquery, but the subquery itself is built by fetching every itemnumber of the set into memory and passing the list as an arrayref that SQL::Abstract renders as one OR clause per itemnumber. For a records with large numbers of items this can produce very large queries. This patch passes the itemnumbers as a subquery so the database resolves the set itself. Existing unit tests should prove there is no change in behavior. Test Plan: 1) Apply this patch 2) prove t/db_dependent/Koha/Items.t 3) Note all tests pass! Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43102 --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 202114 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202114&action=edit Bug 43102: Add POD to _extract_custom_attrs and _process_params Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43102 --- Comment #7 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Hi Kyle, I checked this out and was going to pass QA but Opus has found two things that I think are worth a look. I validated it and it seems these are real: 1) The itemnumber subquery carries the whole resultset, not just the id. get_column('itemnumber')->as_query inherits $self's column list and ORDER BY, so DBIC wraps a derived table rather than emitting a plain id subquery: SELECT me.itemnumber FROM (SELECT <all 48 item columns> FROM items me JOIN biblioitems ... LEFT JOIN branches ... ORDER BY ...) me filter_by_available nests eight of those - a constant 12.7 KB of SQL, even for a record with one item. Trimming the subquery to the one column it needs drops that to 3.9 KB and roughly halves the time again (~30 ms vs ~58 ms at 500 items, median of 3, REST caller shape). 2) The filters now break on a resultset carrying a LIMIT. MariaDB does not allow LIMIT inside an IN subquery: $biblio->items->search({}, { rows => 3 })->filter_by_available->as_list # DBI Exception: DBD::mysql::st execute failed: This version of MariaDB # doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery' All five filters are affected - filter_by_available, filter_by_checked_out, filter_by_in_transit, filter_by_has_holds, filter_by_has_recalls - and all five worked before the patches. No in-tree caller hits it: filters and paging go to the same search() call, so filter_by_* runs unpaged and SUPER::search puts the LIMIT outermost. So it's a public API contract change rather than a live bug, but it fails opaquely, and ->count returns undef instead of propagating the error. Both have the same fix - trim the subquery, then put the derived table back deliberately: $self->_resultset->search( undef, { columns => ['itemnumber'], order_by => undef } ) ->as_subselect_rs->get_column('itemnumber')->as_query on all five occurrences. as_subselect_rs is what keeps LIMIT legal, the column trim is what shrinks the query. Tested with that applied as real code: the same 93 tests pass, available-item sets are identical to the pre-patch behaviour across 50 randomised comparisons, and all five LIMIT cases above behave as they did before. Careful if you try this: dropping the ORDER BY without adding as_subselect_rs makes the LIMIT problem worse. The inherited ORDER BY is forcing the derived-table wrap by accident today, and it's the only reason the search_ordered shape still works - remove it and that case fails too. -- You are receiving this mail because: You are watching all bug changes.