Anonymous | Login
Project:
RSS
  
News | My View | View Issues | Roadmap | Summary

View Issue Details[ Jump to Notes ] [ Issue History ] [ Print ]
ID
0033771
TypeCategorySeverityReproducibilityDate SubmittedLast Update
defect[Openbravo ERP] 07. Sales managementminorhave not tried2016-08-23 17:132016-09-01 12:12
ReportershuehnerView Statuspublic 
Assigned Toshuehner 
PrioritynormalResolutionfixedFixed in Version3.0PR16Q4
StatusclosedFix in branchFixed in SCM revisione23c586b8dbf
ProjectionnoneETAnoneTarget Version
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionSCM revision 
Review Assigned Toaferraz
Web browser
ModulesCore
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary

0033771: Accidental double query in SE_Invoice_BPartner.isAutomaticCombination

DescriptionCode in that method triggers double sql query by calling .list() twice on same OBCriteria instance:

          if (obc.list() == null || obc.list().size() == 0) {

Note: Afaik that '== null' should not be possible in practice, so that should be rechecked and that 2nd that part probably removed.

Also: the filter combination seems to be unique as per this db constraint:

    "fin_finacc_paymentmethod_un" UNIQUE CONSTRAINT, btree (fin_paymentmethod_id, fin_financial_account_id)

So a single line note in java would be nice to have clear that .list() will never load huge results into mem.
Steps To Reproducewith debugging code from: https://issues.openbravo.com/view.php?id=33767 [^]

Open Sales invoice window
new in form view
select a bpartner from dropdown list of selector
-> see double .list() call happen
TagsPerformance
Attached Files

- Relationships [ Relation Graph ] [ Dependency Graph ]
related to feature request 0033767 closedplatform Add code to auto-detect 'accidental double query' on same OBQuery or OBCriteria object 
related to design defect 0036898 newTriage Finance Performance issues when using DAL 

-  Notes
(0089346)
hgbot (developer)
2016-08-24 14:39

 Repository: erp/devel/pi
Changeset: e23c586b8dbf38c79bca558dfdb10a4bbc0f5f1e
Author: Stefan Hühner <stefan.huehner <at> openbravo.com>
Date: Wed Aug 24 14:39:03 2016 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/e23c586b8dbf38c79bca558dfdb10a4bbc0f5f1e [^]

Fixed 33771. Avoid double .list() call to not run query twice.

Old code ran following a OBCriteria
  if (obc.list() == null || obc.list().size() == 0) {

However .list() does not return null as per Hibernate javadoc but empty or
non-empty list (of method raising an exception).

So remove the extra null check to avoid double .list() call.

Also improve .list().size() check slightly by using .list().isEmpty()

Add note that filter is on unique constraint so .list().size()<=1 also

---
M src/org/openbravo/erpCommon/ad_callouts/SE_Invoice_BPartner.java
---
(0089590)
hgbot (developer)
2016-08-31 11:12

 Repository: erp/devel/pi
Changeset: e909d7e193e55e2b4b9163d4120cf04113bce9b1
Author: Alvaro Ferraz <alvaro.ferraz <at> openbravo.com>
Date: Wed Aug 31 11:11:13 2016 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/e909d7e193e55e2b4b9163d4120cf04113bce9b1 [^]

Related to issue 33771: Code review improvements

Use uniqueResult() == null instead of list().isEmpty(), adding setMaxResults(1) to avoid exceptions.
Update copyright.

---
M src/org/openbravo/erpCommon/ad_callouts/SE_Invoice_BPartner.java
---
(0089592)
hgbot (developer)
2016-08-31 11:26

 Repository: erp/devel/pi
Changeset: 43453b625a063992658b0b391e42c9b263fccbc1
Author: Alvaro Ferraz <alvaro.ferraz <at> openbravo.com>
Date: Wed Aug 31 11:25:47 2016 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/43453b625a063992658b0b391e42c9b263fccbc1 [^]

Related to issue 33771: Remove setMaxResults(1)

setMaxResults(1) is not needed as we are filtering by unique constraint.

---
M src/org/openbravo/erpCommon/ad_callouts/SE_Invoice_BPartner.java
---
(0089593)
aferraz (manager)
2016-08-31 11:26

 Code review OK
(0089648)
hudsonbot (developer)
2016-09-01 12:12

 A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/a321ec570edd [^]
Maturity status: Test
(0089649)
hudsonbot (developer)
2016-09-01 12:12

 A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/a321ec570edd [^]
Maturity status: Test

- Issue History
Date Modified Username Field Change
2016-08-23 17:13 shuehner New Issue
2016-08-23 17:13 shuehner Assigned To => Triage Finance
2016-08-23 17:13 shuehner Modules => Core
2016-08-23 17:13 shuehner Triggers an Emergency Pack => No
2016-08-23 17:14 shuehner Tag Attached: Performance
2016-08-23 17:14 shuehner Relationship added related to 0033767
2016-08-24 14:39 hgbot Checkin
2016-08-24 14:39 hgbot Note Added: 0089346
2016-08-24 14:39 hgbot Status new => resolved
2016-08-24 14:39 hgbot Resolution open => fixed
2016-08-24 14:39 hgbot Fixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/e23c586b8dbf38c79bca558dfdb10a4bbc0f5f1e [^]
2016-08-24 14:40 shuehner Assigned To Triage Finance => shuehner
2016-08-24 14:40 shuehner Review Assigned To => aferraz
2016-08-31 11:12 hgbot Checkin
2016-08-31 11:12 hgbot Note Added: 0089590
2016-08-31 11:26 hgbot Checkin
2016-08-31 11:26 hgbot Note Added: 0089592
2016-08-31 11:26 aferraz Note Added: 0089593
2016-08-31 11:26 aferraz Status resolved => closed
2016-08-31 11:26 aferraz Fixed in Version => 3.0PR16Q4
2016-09-01 12:12 hudsonbot Checkin
2016-09-01 12:12 hudsonbot Note Added: 0089648
2016-09-01 12:12 hudsonbot Checkin
2016-09-01 12:12 hudsonbot Note Added: 0089649
2017-09-19 18:47 markmm82 Relationship added related to 0036898

Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker