Bug 35361 - Search filter on receive orders page crashes the system
Summary: Search filter on receive orders page crashes the system
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-17 16:01 UTC by Andreas Jonsson
Modified: 2024-02-16 15:54 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 35361: Replace prefetch with join in REST API. (793 bytes, patch)
2023-11-23 20:10 UTC, Andreas Jonsson
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Jonsson 2023-11-17 16:01:50 UTC
The search filter for the table on the page acqui/parcel.pl generates the below api-request, which results in the below sql query.  Note how the LIMIT clause is applied to the sub-query, not the whole query.  For us, this specific query actually yielded a search result of in total over 171 000 rows.

GET /api/v1/acquisitions/orders?only_active=1&_page=1&_per_page=20&q={"-and":[{"basket.vendor_id":5},[[{"basket.name":{"like":"%97891%"}}],[{"basket.basket_group.name":{"like":"%97891%"}}],[{"me.order_id":{"like":"%97891%"}}],[{"biblio.title":{"like":"%97891%"}},{"biblio.author":{"like":"%97891%"}},{"biblio.isbn":{"like":"%97891%"}},{"biblio.publisher":{"like":"%97891%"}},{"me.internal_note":{"like":"%97891%"}},{"me.vendor_note":{"like":"%97891%"}}],[{"me.replacement_price":{"like":"%97891%"}}],[{"me.quantity":{"like":"%97891%"}}],[{"me.ecost":{"like":"%97891%"}}],[{"fund.name":{"like":"%97891%"}}]]]}&_match=contains&_order_by=+basket.name HTTP/1.1

SELECT `me`.`ordernumber`, `me`.`biblionumber`, `me`.`deleted_biblionumber`, `me`.`entrydate`, `me`.`quantity`, `me`.`currency`, `me`.`listprice`, `me`.`datereceived`, `me`.`invoiceid`, `me`.`freight`, `me`.`unitprice`, `me`.`unitprice_tax_excluded`, `me`.`unitprice_tax_included`, `me`.`quantityreceived`, `me`.`created_by`, `me`.`datecancellationprinted`, `me`.`cancellationreason`, `me`.`order_internalnote`, `me`.`order_vendornote`, `me`.`purchaseordernumber`, `me`.`basketno`, `me`.`timestamp`, `me`.`rrp`, `me`.`replacementprice`, `me`.`rrp_tax_excluded`, `me`.`rrp_tax_included`, `me`.`ecost`, `me`.`ecost_tax_excluded`, `me`.`ecost_tax_included`, `me`.`tax_rate_bak`, `me`.`tax_rate_on_ordering`, `me`.`tax_rate_on_receiving`, `me`.`tax_value_bak`, `me`.`tax_value_on_ordering`, `me`.`tax_value_on_receiving`, `me`.`discount`, `me`.`budget_id`, `me`.`budgetdate`, `me`.`sort1`, `me`.`sort2`, `me`.`sort1_authcat`, `me`.`sort2_authcat`, `me`.`uncertainprice`, `me`.`subscriptionid`, `me`.`parent_ordernumber`, `me`.`orderstatus`, `me`.`line_item_id`, `me`.`suppliers_reference_number`, `me`.`suppliers_reference_qualifier`, `me`.`suppliers_report`, `me`.`estimated_delivery_date`, `me`.`invoice_unitprice`, `me`.`invoice_currency`, `basket`.`basketno`, `basket`.`basketname`, `basket`.`note`, `basket`.`booksellernote`, `basket`.`contractnumber`, `basket`.`creationdate`, `basket`.`closedate`, `basket`.`booksellerid`, `basket`.`authorisedby`, `basket`.`booksellerinvoicenumber`, `basket`.`basketgroupid`, `basket`.`deliveryplace`, `basket`.`billingplace`, `basket`.`branch`, `basket`.`is_standing`, `basket`.`create_items`, `basket_group`.`id`, `basket_group`.`name`, `basket_group`.`closed`, `basket_group`.`booksellerid`, `basket_group`.`deliveryplace`, `basket_group`.`freedeliveryplace`, `basket_group`.`deliverycomment`, `basket_group`.`billingplace`, `biblio`.`biblionumber`, `biblio`.`frameworkcode`, `biblio`.`author`, `biblio`.`title`, `biblio`.`medium`, `biblio`.`subtitle`, `biblio`.`part_number`, `biblio`.`part_name`, `biblio`.`unititle`, `biblio`.`notes`, `biblio`.`serial`, `biblio`.`seriestitle`, `biblio`.`copyrightdate`, `biblio`.`timestamp`, `biblio`.`datecreated`, `biblio`.`abstract`, `biblioitem`.`biblioitemnumber`, `biblioitem`.`biblionumber`, `biblioitem`.`volume`, `biblioitem`.`number`, `biblioitem`.`itemtype`, `biblioitem`.`isbn`, `biblioitem`.`issn`, `biblioitem`.`ean`, `biblioitem`.`publicationyear`, `biblioitem`.`publishercode`, `biblioitem`.`volumedate`, `biblioitem`.`volumedesc`, `biblioitem`.`collectiontitle`, `biblioitem`.`collectionissn`, `biblioitem`.`collectionvolume`, `biblioitem`.`editionstatement`, `biblioitem`.`editionresponsibility`, `biblioitem`.`timestamp`, `biblioitem`.`illus`, `biblioitem`.`pages`, `biblioitem`.`notes`, `biblioitem`.`size`, `biblioitem`.`place`, `biblioitem`.`lccn`, `biblioitem`.`url`, `biblioitem`.`cn_source`, `biblioitem`.`cn_class`, `biblioitem`.`cn_item`, `biblioitem`.`cn_suffix`, `biblioitem`.`cn_sort`, `biblioitem`.`agerestriction`, `biblioitem`.`totalissues`, `items`.`itemnumber`, `items`.`biblionumber`, `items`.`biblioitemnumber`, `items`.`barcode`, `items`.`dateaccessioned`, `items`.`booksellerid`, `items`.`homebranch`, `items`.`price`, `items`.`replacementprice`, `items`.`replacementpricedate`, `items`.`datelastborrowed`, `items`.`datelastseen`, `items`.`stack`, `items`.`notforloan`, `items`.`damaged`, `items`.`damaged_on`, `items`.`itemlost`, `items`.`itemlost_on`, `items`.`withdrawn`, `items`.`withdrawn_on`, `items`.`itemcallnumber`, `items`.`coded_location_qualifier`, `items`.`issues`, `items`.`renewals`, `items`.`reserves`, `items`.`restricted`, `items`.`itemnotes`, `items`.`itemnotes_nonpublic`, `items`.`holdingbranch`, `items`.`timestamp`, `items`.`deleted_on`, `items`.`location`, `items`.`permanent_location`, `items`.`onloan`, `items`.`cn_source`, `items`.`cn_sort`, `items`.`ccode`, `items`.`materials`, `items`.`uri`, `items`.`itype`, `items`.`more_subfields_xml`, `items`.`enumchron`, `items`.`copynumber`, `items`.`stocknumber`, `items`.`new_status`, `items`.`exclude_from_local_holds_priority`, `suggestions`.`suggestionid`, `suggestions`.`suggestedby`, `suggestions`.`suggesteddate`, `suggestions`.`managedby`, `suggestions`.`manageddate`, `suggestions`.`acceptedby`, `suggestions`.`accepteddate`, `suggestions`.`rejectedby`, `suggestions`.`rejecteddate`, `suggestions`.`lastmodificationby`, `suggestions`.`lastmodificationdate`, `suggestions`.`STATUS`, `suggestions`.`archived`, `suggestions`.`note`, `suggestions`.`staff_note`, `suggestions`.`author`, `suggestions`.`title`, `suggestions`.`copyrightdate`, `suggestions`.`publishercode`, `suggestions`.`date`, `suggestions`.`volumedesc`, `suggestions`.`publicationyear`, `suggestions`.`place`, `suggestions`.`isbn`, `suggestions`.`biblionumber`, `suggestions`.`reason`, `suggestions`.`patronreason`, `suggestions`.`budgetid`, `suggestions`.`branchcode`, `suggestions`.`collectiontitle`, `suggestions`.`itemtype`, `suggestions`.`quantity`, `suggestions`.`currency`, `suggestions`.`price`, `suggestions`.`total`, `suggester`.`borrowernumber`, `suggester`.`cardnumber`, `suggester`.`surname`, `suggester`.`firstname`, `suggester`.`middle_name`, `suggester`.`title`, `suggester`.`othernames`, `suggester`.`initials`, `suggester`.`pronouns`, `suggester`.`streetnumber`, `suggester`.`streettype`, `suggester`.`address`, `suggester`.`address2`, `suggester`.`city`, `suggester`.`state`, `suggester`.`zipcode`, `suggester`.`country`, `suggester`.`email`, `suggester`.`phone`, `suggester`.`mobile`, `suggester`.`fax`, `suggester`.`emailpro`, `suggester`.`phonepro`, `suggester`.`B_streetnumber`, `suggester`.`B_streettype`, `suggester`.`B_address`, `suggester`.`B_address2`, `suggester`.`B_city`, `suggester`.`B_state`, `suggester`.`B_zipcode`, `suggester`.`B_country`, `suggester`.`B_email`, `suggester`.`B_phone`, `suggester`.`dateofbirth`, `suggester`.`branchcode`, `suggester`.`categorycode`, `suggester`.`dateenrolled`, `suggester`.`dateexpiry`, `suggester`.`password_expiration_date`, `suggester`.`date_renewed`, `suggester`.`gonenoaddress`, `suggester`.`lost`, `suggester`.`debarred`, `suggester`.`debarredcomment`, `suggester`.`contactname`, `suggester`.`contactfirstname`, `suggester`.`contacttitle`, `suggester`.`borrowernotes`, `suggester`.`relationship`, `suggester`.`sex`, `suggester`.`password`, `suggester`.`secret`, `suggester`.`auth_method`, `suggester`.`flags`, `suggester`.`userid`, `suggester`.`opacnote`, `suggester`.`contactnote`, `suggester`.`sort1`, `suggester`.`sort2`, `suggester`.`altcontactfirstname`, `suggester`.`altcontactsurname`, `suggester`.`altcontactaddress1`, `suggester`.`altcontactaddress2`, `suggester`.`altcontactaddress3`, `suggester`.`altcontactstate`, `suggester`.`altcontactzipcode`, `suggester`.`altcontactcountry`, `suggester`.`altcontactphone`, `suggester`.`smsalertnumber`, `suggester`.`sms_provider_id`, `suggester`.`privacy`, `suggester`.`privacy_guarantor_fines`, `suggester`.`privacy_guarantor_checkouts`, `suggester`.`checkprevcheckout`, `suggester`.`updated_on`, `suggester`.`lastseen`, `suggester`.`lang`, `suggester`.`login_attempts`, `suggester`.`overdrive_auth_token`, `suggester`.`anonymized`, `suggester`.`autorenew_checkouts`, `suggester`.`primary_contact_method`, `fund`.`budget_id`, `fund`.`budget_parent_id`, `fund`.`budget_code`, `fund`.`budget_name`, `fund`.`budget_branchcode`, `fund`.`budget_amount`, `fund`.`budget_encumb`, `fund`.`budget_expend`, `fund`.`budget_notes`, `fund`.`timestamp`, `fund`.`budget_period_id`, `fund`.`sort1_authcat`, `fund`.`sort2_authcat`, `fund`.`budget_owner_id`, `fund`.`budget_permission` FROM (SELECT `me`.`ordernumber`, `me`.`biblionumber`, `me`.`deleted_biblionumber`, `me`.`entrydate`, `me`.`quantity`, `me`.`currency`, `me`.`listprice`, `me`.`datereceived`, `me`.`invoiceid`, `me`.`freight`, `me`.`unitprice`, `me`.`unitprice_tax_excluded`, `me`.`unitprice_tax_included`, `me`.`quantityreceived`, `me`.`created_by`, `me`.`datecancellationprinted`, `me`.`cancellationreason`, `me`.`order_internalnote`, `me`.`order_vendornote`, `me`.`purchaseordernumber`, `me`.`basketno`, `me`.`timestamp`, `me`.`rrp`, `me`.`replacementprice`, `me`.`rrp_tax_excluded`, `me`.`rrp_tax_included`, `me`.`ecost`, `me`.`ecost_tax_excluded`, `me`.`ecost_tax_included`, `me`.`tax_rate_bak`, `me`.`tax_rate_on_ordering`, `me`.`tax_rate_on_receiving`, `me`.`tax_value_bak`, `me`.`tax_value_on_ordering`, `me`.`tax_value_on_receiving`, `me`.`discount`, `me`.`budget_id`, `me`.`budgetdate`, `me`.`sort1`, `me`.`sort2`, `me`.`sort1_authcat`, `me`.`sort2_authcat`, `me`.`uncertainprice`, `me`.`subscriptionid`, `me`.`parent_ordernumber`, `me`.`orderstatus`, `me`.`line_item_id`, `me`.`suppliers_reference_number`, `me`.`suppliers_reference_qualifier`, `me`.`suppliers_report`, `me`.`estimated_delivery_date`, `me`.`invoice_unitprice`, `me`.`invoice_currency` FROM `aqorders` `me` LEFT JOIN `aqbasket` `basket` ON `basket`.`basketno` = `me`.`basketno` LEFT JOIN `aqbasketgroups` `basket_group` ON `basket_group`.`id` = `basket`.`basketgroupid` LEFT JOIN `biblio` `biblio` ON `biblio`.`biblionumber` = `me`.`biblionumber` LEFT JOIN `biblioitems` `biblioitem` ON `biblioitem`.`biblionumber` = `biblio`.`biblionumber`  JOIN `aqbudgets` `fund` ON `fund`.`budget_id` = `me`.`budget_id` WHERE ( ( ( ( `me`.`ecost` LIKE '%97891%' OR `me`.`quantity` LIKE '%97891%' OR `me`.`ordernumber` LIKE '%97891%' OR `fund`.`budget_name` LIKE '%97891%' OR `basket`.`basketname` LIKE '%97891%' OR `basket_group`.`name` LIKE '%97891%' OR `me`.`replacementprice` LIKE '%97891%' OR `biblio`.`title` LIKE '%97891%' OR `biblio`.`author` LIKE '%97891%' OR `biblioitem`.`isbn` LIKE '%97891%' OR `me`.`order_vendornote` LIKE '%97891%' OR `me`.`order_internalnote` LIKE '%97891%' OR `biblioitem`.`publishercode` LIKE '%97891%' ) AND ( ( `orderstatus` = 'ordered' OR `orderstatus` = 'partial' ) OR ( `basket`.`is_standing` = '1' AND ( `orderstatus` = 'new' OR `orderstatus` = 'ordered' OR `orderstatus` = 'partial' ) ) ) ) AND `basket`.`booksellerid` = '5' ) ) GROUP BY `me`.`ordernumber`, `me`.`biblionumber`, `me`.`deleted_biblionumber`, `me`.`entrydate`, `me`.`quantity`, `me`.`currency`, `me`.`listprice`, `me`.`datereceived`, `me`.`invoiceid`, `me`.`freight`, `me`.`unitprice`, `me`.`unitprice_tax_excluded`, `me`.`unitprice_tax_included`, `me`.`quantityreceived`, `me`.`created_by`, `me`.`datecancellationprinted`, `me`.`cancellationreason`, `me`.`order_internalnote`, `me`.`order_vendornote`, `me`.`purchaseordernumber`, `me`.`basketno`, `me`.`timestamp`, `me`.`rrp`, `me`.`replacementprice`, `me`.`rrp_tax_excluded`, `me`.`rrp_tax_included`, `me`.`ecost`, `me`.`ecost_tax_excluded`, `me`.`ecost_tax_included`, `me`.`tax_rate_bak`, `me`.`tax_rate_on_ordering`, `me`.`tax_rate_on_receiving`, `me`.`tax_value_bak`, `me`.`tax_value_on_ordering`, `me`.`tax_value_on_receiving`, `me`.`discount`, `me`.`budget_id`, `me`.`budgetdate`, `me`.`sort1`, `me`.`sort2`, `me`.`sort1_authcat`, `me`.`sort2_authcat`, `me`.`uncertainprice`, `me`.`subscriptionid`, `me`.`parent_ordernumber`, `me`.`orderstatus`, `me`.`line_item_id`, `me`.`suppliers_reference_number`, `me`.`suppliers_reference_qualifier`, `me`.`suppliers_report`, `me`.`estimated_delivery_date`, `me`.`invoice_unitprice`, `me`.`invoice_currency`, `basket`.`basketname` ORDER BY `basket`.`basketname` ASC LIMIT 20) `me` LEFT JOIN `aqbasket` `basket` ON `basket`.`basketno` = `me`.`basketno` LEFT JOIN `aqbasketgroups` `basket_group` ON `basket_group`.`id` = `basket`.`basketgroupid` LEFT JOIN `biblio` `biblio` ON `biblio`.`biblionumber` = `me`.`biblionumber` LEFT JOIN `biblioitems` `biblioitem` ON `biblioitem`.`biblionumber` = `biblio`.`biblionumber` LEFT JOIN `items` `items` ON `items`.`biblionumber` = `biblio`.`biblionumber` LEFT JOIN `suggestions` `suggestions` ON `suggestions`.`biblionumber` = `biblio`.`biblionumber` LEFT JOIN `borrowers` `suggester` ON `suggester`.`borrowernumber` = `suggestions`.`suggestedby`  JOIN `aqbudgets` `fund` ON `fund`.`budget_id` = `me`.`budget_id` WHERE ( ( ( ( `me`.`ecost` LIKE '%97891%' OR `me`.`quantity` LIKE '%97891%' OR `me`.`ordernumber` LIKE '%97891%' OR `fund`.`budget_name` LIKE '%97891%' OR `basket`.`basketname` LIKE '%97891%' OR `basket_group`.`name` LIKE '%97891%' OR `me`.`replacementprice` LIKE '%97891%' OR `biblio`.`title` LIKE '%97891%' OR `biblio`.`author` LIKE '%97891%' OR `biblioitem`.`isbn` LIKE '%97891%' OR `me`.`order_vendornote` LIKE '%97891%' OR `me`.`order_internalnote` LIKE '%97891%' OR `biblioitem`.`publishercode` LIKE '%97891%' ) AND ( ( `orderstatus` = 'ordered' OR `orderstatus` = 'partial' ) OR ( `basket`.`is_standing` = '1' AND ( `orderstatus` = 'new' OR `orderstatus` = 'ordered' OR `orderstatus` = 'partial' ) ) ) ) AND `basket`.`booksellerid` = '5' ) ) ORDER BY `basket`.`basketname` ASC;
Comment 1 Andreas Jonsson 2023-11-23 14:37:35 UTC
This is just an instance of a much bigger problem with DBIx.

For instance the SQL code of an ordinary borrower search looks like this, once again, the LIMIT-clause is applied on the subquery:

SELECT `me`.`borrowernumber`, `me`.`cardnumber`, `me`.`surname`, `me`.`firstname`, `me`.`middle_name`, `me`.`title`, `me`.`othernames`, `me`.`initials`, `me`.`pronouns`, `me`.`streetnumber`, `me`.`streettype`, `me`.`address`, `me`.`address2`, `me`.`city`, `me`.`state`, `me`.`zipcode`, `me`.`country`, `me`.`email`, `me`.`phone`, `me`.`mobile`, `me`.`fax`, `me`.`emailpro`, `me`.`phonepro`, `me`.`B_streetnumber`, `me`.`B_streettype`, `me`.`B_address`, `me`.`B_address2`, `me`.`B_city`, `me`.`B_state`, `me`.`B_zipcode`, `me`.`B_country`, `me`.`B_email`, `me`.`B_phone`, `me`.`dateofbirth`, `me`.`branchcode`, `me`.`categorycode`, `me`.`dateenrolled`, `me`.`dateexpiry`, `me`.`password_expiration_date`, `me`.`date_renewed`, `me`.`gonenoaddress`, `me`.`lost`, `me`.`debarred`, `me`.`debarredcomment`, `me`.`contactname`, `me`.`contactfirstname`, `me`.`contacttitle`, `me`.`borrowernotes`, `me`.`relationship`, `me`.`sex`, `me`.`password`, `me`.`secret`, `me`.`auth_method`, `me`.`flags`, `me`.`userid`, `me`.`opacnote`, `me`.`contactnote`, `me`.`sort1`, `me`.`sort2`, `me`.`altcontactfirstname`, `me`.`altcontactsurname`, `me`.`altcontactaddress1`, `me`.`altcontactaddress2`, `me`.`altcontactaddress3`, `me`.`altcontactstate`, `me`.`altcontactzipcode`, `me`.`altcontactcountry`, `me`.`altcontactphone`, `me`.`smsalertnumber`, `me`.`sms_provider_id`, `me`.`privacy`, `me`.`privacy_guarantor_fines`, `me`.`privacy_guarantor_checkouts`, `me`.`checkprevcheckout`, `me`.`updated_on`, `me`.`lastseen`, `me`.`lang`, `me`.`login_attempts`, `me`.`overdrive_auth_token`, `me`.`anonymized`, `me`.`autorenew_checkouts`, `me`.`primary_contact_method`, `extended_attributes`.`id`, `extended_attributes`.`borrowernumber`, `extended_attributes`.`code`, `extended_attributes`.`attribute` FROM (SELECT `me`.`borrowernumber`, `me`.`cardnumber`, `me`.`surname`, `me`.`firstname`, `me`.`middle_name`, `me`.`title`, `me`.`othernames`, `me`.`initials`, `me`.`pronouns`, `me`.`streetnumber`, `me`.`streettype`, `me`.`address`, `me`.`address2`, `me`.`city`, `me`.`state`, `me`.`zipcode`, `me`.`country`, `me`.`email`, `me`.`phone`, `me`.`mobile`, `me`.`fax`, `me`.`emailpro`, `me`.`phonepro`, `me`.`B_streetnumber`, `me`.`B_streettype`, `me`.`B_address`, `me`.`B_address2`, `me`.`B_city`, `me`.`B_state`, `me`.`B_zipcode`, `me`.`B_country`, `me`.`B_email`, `me`.`B_phone`, `me`.`dateofbirth`, `me`.`branchcode`, `me`.`categorycode`, `me`.`dateenrolled`, `me`.`dateexpiry`, `me`.`password_expiration_date`, `me`.`date_renewed`, `me`.`gonenoaddress`, `me`.`lost`, `me`.`debarred`, `me`.`debarredcomment`, `me`.`contactname`, `me`.`contactfirstname`, `me`.`contacttitle`, `me`.`borrowernotes`, `me`.`relationship`, `me`.`sex`, `me`.`password`, `me`.`secret`, `me`.`auth_method`, `me`.`flags`, `me`.`userid`, `me`.`opacnote`, `me`.`contactnote`, `me`.`sort1`, `me`.`sort2`, `me`.`altcontactfirstname`, `me`.`altcontactsurname`, `me`.`altcontactaddress1`, `me`.`altcontactaddress2`, `me`.`altcontactaddress3`, `me`.`altcontactstate`, `me`.`altcontactzipcode`, `me`.`altcontactcountry`, `me`.`altcontactphone`, `me`.`smsalertnumber`, `me`.`sms_provider_id`, `me`.`privacy`, `me`.`privacy_guarantor_fines`, `me`.`privacy_guarantor_checkouts`, `me`.`checkprevcheckout`, `me`.`updated_on`, `me`.`lastseen`, `me`.`lang`, `me`.`login_attempts`, `me`.`overdrive_auth_token`, `me`.`anonymized`, `me`.`autorenew_checkouts`, `me`.`primary_contact_method` FROM `borrowers` `me` LEFT JOIN `borrower_attributes` `extended_attributes` ON `extended_attributes`.`borrowernumber` = `me`.`borrowernumber` WHERE ( ( ( `me`.`borrowernumber` LIKE '%nil%' OR `me`.`email` LIKE '%nil%' OR `me`.`surname` LIKE '%nil%' OR `me`.`firstname` LIKE '%nil%' OR `me`.`othernames` LIKE '%nil%' OR `me`.`cardnumber` LIKE '%nil%' OR `me`.`userid` LIKE '%nil%' OR `me`.`B_address` LIKE '%nil%' OR `me`.`middle_name` LIKE '%nil%' ) OR ( `me`.`borrowernumber` LIKE '%nil%' OR `me`.`email` LIKE '%nil%' OR `me`.`surname` LIKE '%nil%' OR `me`.`firstname` LIKE '%nil%' OR `me`.`othernames` LIKE '%nil%' OR `me`.`cardnumber` LIKE '%nil%' OR `me`.`userid` LIKE '%nil%' OR `me`.`B_address` LIKE '%nil%' OR `me`.`middle_name` LIKE '%nil%' ) OR ( `extended_attributes`.`attribute` LIKE '%nil%' AND ( `extended_attributes`.`code` = 'BUSS' OR `extended_attributes`.`code` = 'MEROPPET' ) ) ) ) GROUP BY `me`.`borrowernumber`, `me`.`cardnumber`, `me`.`surname`, `me`.`firstname`, `me`.`middle_name`, `me`.`title`, `me`.`othernames`, `me`.`initials`, `me`.`pronouns`, `me`.`streetnumber`, `me`.`streettype`, `me`.`address`, `me`.`address2`, `me`.`city`, `me`.`state`, `me`.`zipcode`, `me`.`country`, `me`.`email`, `me`.`phone`, `me`.`mobile`, `me`.`fax`, `me`.`emailpro`, `me`.`phonepro`, `me`.`B_streetnumber`, `me`.`B_streettype`, `me`.`B_address`, `me`.`B_address2`, `me`.`B_city`, `me`.`B_state`, `me`.`B_zipcode`, `me`.`B_country`, `me`.`B_email`, `me`.`B_phone`, `me`.`dateofbirth`, `me`.`branchcode`, `me`.`categorycode`, `me`.`dateenrolled`, `me`.`dateexpiry`, `me`.`password_expiration_date`, `me`.`date_renewed`, `me`.`gonenoaddress`, `me`.`lost`, `me`.`debarred`, `me`.`debarredcomment`, `me`.`contactname`, `me`.`contactfirstname`, `me`.`contacttitle`, `me`.`borrowernotes`, `me`.`relationship`, `me`.`sex`, `me`.`password`, `me`.`secret`, `me`.`auth_method`, `me`.`flags`, `me`.`userid`, `me`.`opacnote`, `me`.`contactnote`, `me`.`sort1`, `me`.`sort2`, `me`.`altcontactfirstname`, `me`.`altcontactsurname`, `me`.`altcontactaddress1`, `me`.`altcontactaddress2`, `me`.`altcontactaddress3`, `me`.`altcontactstate`, `me`.`altcontactzipcode`, `me`.`altcontactcountry`, `me`.`altcontactphone`, `me`.`smsalertnumber`, `me`.`sms_provider_id`, `me`.`privacy`, `me`.`privacy_guarantor_fines`, `me`.`privacy_guarantor_checkouts`, `me`.`checkprevcheckout`, `me`.`updated_on`, `me`.`lastseen`, `me`.`lang`, `me`.`login_attempts`, `me`.`overdrive_auth_token`, `me`.`anonymized`, `me`.`autorenew_checkouts`, `me`.`primary_contact_method` ORDER BY `me`.`surname` ASC, `me`.`firstname` ASC, `me`.`middle_name` ASC, `me`.`othernames` ASC, `me`.`streetnumber` ASC, `me`.`address` ASC, `me`.`address2` ASC, `me`.`city` ASC, `me`.`state` ASC, `me`.`zipcode` ASC, `me`.`country` ASC LIMIT 20) `me` LEFT JOIN `borrower_attributes` `extended_attributes` ON `extended_attributes`.`borrowernumber` = `me`.`borrowernumber` WHERE ( ( ( `me`.`borrowernumber` LIKE '%nil%' OR `me`.`email` LIKE '%nil%' OR `me`.`surname` LIKE '%nil%' OR `me`.`firstname` LIKE '%nil%' OR `me`.`othernames` LIKE '%nil%' OR `me`.`cardnumber` LIKE '%nil%' OR `me`.`userid` LIKE '%nil%' OR `me`.`B_address` LIKE '%nil%' OR `me`.`middle_name` LIKE '%nil%' ) OR ( `me`.`borrowernumber` LIKE '%nil%' OR `me`.`email` LIKE '%nil%' OR `me`.`surname` LIKE '%nil%' OR `me`.`firstname` LIKE '%nil%' OR `me`.`othernames` LIKE '%nil%' OR `me`.`cardnumber` LIKE '%nil%' OR `me`.`userid` LIKE '%nil%' OR `me`.`B_address` LIKE '%nil%' OR `me`.`middle_name` LIKE '%nil%' ) OR ( `extended_attributes`.`attribute` LIKE '%nil%' AND ( `extended_attributes`.`code` = 'BUSS' OR `extended_attributes`.`code` = 'MEROPPET' ) ) ) ) ORDER BY `me`.`surname` ASC, `me`.`firstname` ASC, `me`.`middle_name` ASC, `me`.`othernames` ASC, `me`.`streetnumber` ASC, `me`.`address` ASC, `me`.`address2` ASC, `me`.`city` ASC, `me`.`state` ASC, `me`.`zipcode` ASC, `me`.`country` ASC;
Comment 2 Andreas Jonsson 2023-11-23 17:12:36 UTC
This is apparently a known issue with DBIx: https://rt.cpan.org/Public/Bug/Display.html?id=59764

Open since 2010 with priority "low".

Do we have a plan to move away from DBIx?
Comment 3 Andreas Jonsson 2023-11-23 20:10:47 UTC
Created attachment 159213 [details] [review]
Bug 35361: Replace prefetch with join in REST API.
Comment 4 Andreas Jonsson 2023-11-23 20:19:22 UTC
Replacing "prefetch" with "join" actually solves this problem.   Why are we using "prefetch"?  This is what the documentation has to say:

"Prefetch does a lot of deep magic. As such, it may not behave exactly as you might expect."

On the other hand, the documentation also says:

"...,  only the prefetch technique will deal sanely with fetching related objects over a has_many relation. "
Comment 5 Katrin Fischer 2024-01-22 20:54:46 UTC
(In reply to Andreas Jonsson from comment #4)
> Replacing "prefetch" with "join" actually solves this problem.   Why are we
> using "prefetch"?  This is what the documentation has to say:
> 
> "Prefetch does a lot of deep magic. As such, it may not behave exactly as
> you might expect."

Tomas, can you maybe help with that question?
Comment 6 David Cook 2024-02-14 03:01:31 UTC
(In reply to Andreas Jonsson from comment #0)
> The search filter for the table on the page acqui/parcel.pl generates the
> below api-request, which results in the below sql query.  Note how the LIMIT
> clause is applied to the sub-query, not the whole query.  For us, this
> specific query actually yielded a search result of in total over 171 000
> rows.

This particular query here does seem hypothetical rather than practical. 

The size of the query does make it difficult to boil down. It looks like your vendor must have a large number of orders. In theory, that LIMIT 20 should limit the results, but there are a lot of JOINs across different tables, so you could end up with some unexpected results I suppose...

(In reply to Andreas Jonsson from comment #2)
> Do we have a plan to move away from DBIx?

No, Koha is in deep with DBIx. 

(In reply to Andreas Jonsson from comment #4)
> Replacing "prefetch" with "join" actually solves this problem.   Why are we
> using "prefetch"?  This is what the documentation has to say:
> 
> "Prefetch does a lot of deep magic. As such, it may not behave exactly as
> you might expect."
> 
> On the other hand, the documentation also says:
> 
> "...,  only the prefetch technique will deal sanely with fetching related
> objects over a has_many relation. "

Koha does some interesting generic handling of search queries that get translated into DBIx queries. It might be that Koha is doing something wrong as well.
Comment 7 David Cook 2024-02-14 03:05:34 UTC
Also, can you define "crashes the system"? 

At the moment, I don't know that this one is "critical", because I haven't heard of any systems crashing because of this.

Does the server hang because of the large result set? Is a 500 error thrown?
Comment 8 Andreas Jonsson 2024-02-14 13:39:29 UTC
It is not hypothetical.  If you find an invoice under Acquisitions -> Invoices and then "Go to receipt page" (i.e., acqui/parcel.pl) you will find a table with open orders.  If you type anything into the search filter, the above will be the sql-query generated.

Since there are LEFT JOIN clauses applied after the limit have been applied, and several of those joins have the potential to multiply the number of result lines that are produced, the number of matched lines can explode.

The problem is that all lines are loaded into a perl-process that filters out duplicates.  This perl-filtering consumes heaps of memory and takes several minutes to perform.  Since it is an automatic search performed by typing in the box there will typically be multiple such search requests issued. This makes the system unresponsive and likely triggers the oom killer.

The Libraries in question is using a work flow for their acquisitions where orders are often left opened.  Over time there will be many open orders.  It starts to become problematic at about 20,000 open orders.

Of course, this should not be a problem, because the request should have been limited to return at most 20 rows.

DBIx is fundamentally flawed, and I think it would be a good idea to write our own sql-queries and our own code for generating sql queries.
Comment 9 Andreas Jonsson 2024-02-14 13:42:53 UTC
We are currently working around the problem by regularly creating new vendor objects and registering new orders on the new vendor object to keep the number of opened orders per vendor limited.
Comment 10 David Cook 2024-02-14 22:23:41 UTC
(In reply to Andreas Jonsson from comment #8)
> It is not hypothetical.  If you find an invoice under Acquisitions ->
> Invoices and then "Go to receipt page" (i.e., acqui/parcel.pl) you will find
> a table with open orders.  If you type anything into the search filter, the
> above will be the sql-query generated.

In retrospect, I think that I was testing something different. I was looking just at the column level search filters. I think you must've been table level search filter. My mistake. 

> Since there are LEFT JOIN clauses applied after the limit have been applied,
> and several of those joins have the potential to multiply the number of
> result lines that are produced, the number of matched lines can explode.

Agreed.

> DBIx is fundamentally flawed, and I think it would be a good idea to write
> our own sql-queries and our own code for generating sql queries.

Take a look back in the git log maybe 10 years ago ;). The DBIx ORM is a relatively new addition in the life of Koha. It replaced a lot of bespoke SQL queries. I don't think we'll be going backwards.

However, for complex cases like this one, maybe we do need the controller to have a more specific handler with custom SQL queries.
Comment 11 Andreas Jonsson 2024-02-16 15:54:28 UTC
I can understand why we want to use something like DBIx::Class. But have we properly considered the quality and the viability of DBIx::Class as an open source project? My impression is that it is not a good library:

- The query semantics is vague and ambigous
- The documentation cannot be trusted
- It is slow
- It is unmaintainable

Although hundreds of people are listed as contributors, judging by the commit history, it is in practice a one-man project.

Is there any Koha developer who has read the source code (e.g., https://metacpan.org/release/RIBASUSHI/DBIx-Class-0.082843/source/lib/DBIx/Class/Storage/DBI.pm) and can say that they feel confident about the situation?