Openbravo Issue Tracking System - Retail Modules
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0033139Retail ModulesWeb POSpublic2016-06-05 15:122016-08-18 11:47
Reportershuehner 
Assigned Tomario_castello 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionFixed in VersionRR16Q4 
Merge Request Status
Review Assigned Toguilleaer
OBNetwork customer
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0033139: OrderLoader: Inefficient check for m_locator doing 2-3 queries for the same
DescriptionOrderLoader is querying m_locator (for order.getWarehouse()) essentially up to 3 times

1.) just after 'if (createShipment)' to see if none are present and error out
2.) a few lines later in call to createShipment doing .count() to check if exactly one is present
3.) then just after that said count another .uniqueResult() to retrieve that 'exactly one'

All those can be done with a single sql.
Steps To ReproduceTrace SQL done by OrderLoader.
Proposed SolutionReplace all 3 calls by single query.

Adding simple .setMaxResults(2) and .list() gives us all the info we need and the limit 2 avoids ever reading >2 rows into memory.

Note: Even while only wanting .count()==1 and reading single row we need to use a limit of 2 to distinguish the count=1 or count>1 case.

As we only care about count being exactly 1 (or not) we do not need to do proper count for any values >1 to the limit 2 does the trick nicely.

Additional Information
TagsPerformance
Relationships
depends on backport 0033615RR16Q3.1 closed mario_castello Retail Modules OrderLoader: Inefficient check for m_locator doing 2-3 queries for the same 
depends on backport 0033616RR16Q2.2 closed Retail Retail Modules OrderLoader: Inefficient check for m_locator doing 2-3 queries for the same 
has duplicate defect 0033619 closed Retail Retail Modules Slow order loader process when there are thousends of locators for one warehouse 
related to defect 0033627 closed mtaal Openbravo ERP debug call in OBCriteria.count does trigger extra db query in some case 
blocks defect 0033732RR16Q4 closed guilleaer Retail Modules [API change] Reuse Locator List Informationin createShipmentLines 
Attached Filesdiff 33139.diff (2,601) 2016-06-05 15:13
https://issues.openbravo.com/file_download.php?file_id=9487&type=bug
Issue History
Date ModifiedUsernameFieldChange
2016-06-05 15:12shuehnerNew Issue
2016-06-05 15:12shuehnerAssigned To => Retail
2016-06-05 15:12shuehnerTriggers an Emergency Pack => No
2016-06-05 15:13shuehnerFile Added: 33139.diff
2016-06-05 15:14shuehnerNote Added: 0087003
2016-06-05 15:14shuehnerTag Attached: Performance
2016-08-03 14:26marvintmSeverityminor => major
2016-08-03 14:26marvintmStatusnew => scheduled
2016-08-03 15:54egoitzRelationship addedhas duplicate 0033619
2016-08-03 22:38mario_castelloAssigned ToRetail => mario_castello
2016-08-04 16:05hgbotCheckin
2016-08-04 16:05hgbotNote Added: 0088822
2016-08-04 16:05hgbotStatusscheduled => resolved
2016-08-04 16:05hgbotResolutionopen => fixed
2016-08-04 16:05hgbotFixed in SCM revision => http://code.openbravo.com/erp/pmods/org.openbravo.retail.posterminal/rev/678425057c397e1f5ac3c7ca65244227cbbeb84a [^]
2016-08-04 17:21shuehnerRelationship addedrelated to 0033627
2016-08-10 10:02migueldejuanaReview Assigned To => mtaal
2016-08-16 10:27mtaalNote Added: 0089123
2016-08-16 10:27mtaalStatusresolved => new
2016-08-16 10:27mtaalResolutionfixed => open
2016-08-17 17:20mario_castelloStatusnew => acknowledged
2016-08-17 19:08guilleaerRelationship addedblocks 0033732
2016-08-18 11:46guilleaerNote Added: 0089207
2016-08-18 11:46guilleaerStatusacknowledged => scheduled
2016-08-18 11:47guilleaerStatusscheduled => resolved
2016-08-18 11:47guilleaerFixed in Version => RR16Q4
2016-08-18 11:47guilleaerResolutionopen => fixed
2016-08-18 11:47guilleaerReview Assigned Tomtaal => guilleaer
2016-08-18 11:47guilleaerNote Added: 0089208
2016-08-18 11:47guilleaerStatusresolved => closed

Notes
(0087003)
shuehner   
2016-06-05 15:14   
NOTE: The attached patch does an API change as the method createShipmentLines is protected (so part of API).
(0088822)
hgbot   
2016-08-04 16:05   
Repository: erp/pmods/org.openbravo.retail.posterminal
Changeset: 678425057c397e1f5ac3c7ca65244227cbbeb84a
Author: Mario Castello <mario.castello <at> peoplewalking.com>
Date: Wed Aug 03 14:28:43 2016 -0600
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.posterminal/rev/678425057c397e1f5ac3c7ca65244227cbbeb84a [^]

Fixed issue 33139: Replace the sames query for m_locator in orderloader class for only one query

---
M src/org/openbravo/retail/posterminal/OrderLoader.java
---
(0089123)
mtaal   
2016-08-16 10:27   
Hi,
Officially this is an api change which is not backward compatible by changing the signature of the protected method.

An api change needs to be approved by dmitry (just drop him an email with the link to this issue) and then add it on this page:
http://wiki.openbravo.com/wiki/Retail:API_changes [^]

In the PI section.

gr. Martin
(0089207)
guilleaer   
2016-08-18 11:46   
API check approved

https://issues.openbravo.com/view.php?id=33732 [^]

Added to API changes wiki page

http://wiki.openbravo.com/wiki/Retail:API_changes [^]
(0089208)
guilleaer   
2016-08-18 11:47   
Closed and Reviewed. API change approved