details: https://code.openbravo.com/erp/devel/pi/rev/a5058ea3c279 changeset: 31582:a5058ea3c279 user: Víctor Martínez Romanos <victor.martinez <at> openbravo.com> date: Thu Mar 02 14:03:01 2017 +0100 summary: Fixed bug 32493: Pareto Product Report performance refactor
The Pareto Product report had very important performance issues due to the way data was retrieved from the database. Multiple (and unnecessary) sequencial scans were executed on high volume tables making the report unusable on real environments. Besides it has been detected that the information shown was not right: products without stock might appear in the report, the percentages were clearly not right and the ABC category was also wrong because it was based on wrong percentages. The fix mainly includes a total refactor of the queries, which fixes most of the performance issues. It also adds data aggregation support based on Valued Stock aggregated data, which should help to keep the report's performance when the time goes by. It has been tested that, even without aggregated data, the report performs really well on high volume environments. Example: An environment without aggregated data and with 7.7E+6 transactions took only 38 seconds to get the data for an organization with 250E+3 transactions; before it took "days". List of important changes: * Deprecated M_GET_PARETO_ABC function and implemented the same logic directly into the SQL query * Added index on M_TRANSACTION_COST table to the DATEACCT column, which is heavily used as a filter criteria for many reports (included Pareto Product) * Modified index M_VALUED_STOCK_AGG_DATETO on M_VALUED_STOCK_AGG to take into account AD_ORG_ID and DATETO. This makes the queries to get data from this aggregated table to perform really well. * Modified ReportParetoProduct_data.xsq select query. This is actually the key to get the performance improvement: * Usage of window functions to calculate: * the total value per organization's warehouse, * the percentage per product and organization's warehouse, * the accumulated percentage per product and organization's warehouse. * The ABC is calculated looking at the accumulated percentage gotten before instead of calling the M_GET_PARETO_ABC db function. * Reduced the usage of functions in select clause because, although individually they are quite fast, they must be executed over multiple records thus making the whole query really slow (example 2ms multiplied by 10E3 records delays the query 20 seconds!): * ad_get_org_le_bu is not needed anymore because the legal entity will be always the same for each record in the report. Note that the Pareto's organization combo only shows legal entities or child organizations (which only have a unique legal entity). * c_currency_convert_precision is only called when a currency conversion is needed, thus reducing unnecessary overhead. * ad_column_identifier calls have been removed. Instead we get static (translated) values. * Transactions are now filtered by trxprocessdate when the costing rule was started, so we avoid to compute legacy records. * Although not strictly necessary to get a performance improvement, the query has been split in several CTEs, thus making the query more readable and easy to maintain. details: https://code.openbravo.com/erp/devel/pi/rev/9b9243b31573 changeset: 31583:9b9243b31573 user: Víctor Martínez Romanos <victor.martinez <at> openbravo.com> date: Thu Mar 02 14:25:51 2017 +0100 summary: Fixed issue 35410: Default currency in Pareto Product Report Set default currency according to the selected organization details: https://code.openbravo.com/erp/devel/pi/rev/205a911b298f changeset: 31584:205a911b298f user: David Miguelez <david.miguelez <at> openbravo.com> date: Fri Mar 03 12:48:01 2017 +0100 summary: Related to Issue 35410. Code Review. Fixes problem when retrieving currency from parameter window. Also, fixes problem of currency reloading when no organization has been changed. diffstat: src-db/database/model/functions/M_GET_PARETO_ABC.xml | 8 +- src-db/database/model/tables/M_TRANSACTION_COST.xml | 3 + src-db/database/model/tables/M_VALUED_STOCK_AGG.xml | 7 +- src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct.html | 28 +- src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct.java | 72 +++- src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct_data.xsql | 205 ++++++--- src/org/openbravo/erpCommon/ad_reports/ReportValuationStock.java | 2 +- 7 files changed, 250 insertions(+), 75 deletions(-) diffs (truncated from 488 to 300 lines): diff -r e349ce84e231 -r 205a911b298f src-db/database/model/functions/M_GET_PARETO_ABC.xml --- a/src-db/database/model/functions/M_GET_PARETO_ABC.xml Fri Mar 03 10:49:49 2017 +0100 +++ b/src-db/database/model/functions/M_GET_PARETO_ABC.xml Fri Mar 03 12:48:01 2017 +0100 @@ -28,11 +28,17 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU -* All portions are Copyright (C) 2001-2016 Openbravo SLU +* All portions are Copyright (C) 2001-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************/ +/* +* This code has very important performance issues and it has been deprecated. +* ReportParetoProduct_data.xsql, select method has been rewritten so it is not needed anymore. +* @deprecated +*/ + VARaCUM NUMBER:=0; VARaCUMB NUMBER:=0; v_limitA NUMBER:=80; diff -r e349ce84e231 -r 205a911b298f src-db/database/model/tables/M_TRANSACTION_COST.xml --- a/src-db/database/model/tables/M_TRANSACTION_COST.xml Fri Mar 03 10:49:49 2017 +0100 +++ b/src-db/database/model/tables/M_TRANSACTION_COST.xml Fri Mar 03 12:48:01 2017 +0100 @@ -76,6 +76,9 @@ <foreign-key foreignTable="C_CURRENCY" name="M_TRANSCOST_CURRENCY"> <reference local="C_CURRENCY_ID" foreign="C_CURRENCY_ID"/> </foreign-key> + <index name="M_TRANSACTION_COST_DATEACCT" unique="false"> + <index-column name="DATEACCT"/> + </index> <index name="M_TRANSACTION_COST_TRX" unique="false"> <index-column name="M_TRANSACTION_ID"/> </index> diff -r e349ce84e231 -r 205a911b298f src-db/database/model/tables/M_VALUED_STOCK_AGG.xml --- a/src-db/database/model/tables/M_VALUED_STOCK_AGG.xml Fri Mar 03 10:49:49 2017 +0100 +++ b/src-db/database/model/tables/M_VALUED_STOCK_AGG.xml Fri Mar 03 12:48:01 2017 +0100 @@ -106,12 +106,13 @@ <index name="M_VALUED_STOCK_AGG_DATEFROM" unique="false"> <index-column name="DATEFROM"/> </index> - <index name="M_VALUED_STOCK_AGG_DATETO" unique="false"> - <index-column name="DATETO"/> - </index> <index name="M_VALUED_STOCK_AGG_LOCATOR" unique="false"> <index-column name="M_LOCATOR_ID"/> </index> + <index name="M_VALUED_STOCK_AGG_ORG_DATETO" unique="false"> + <index-column name="AD_ORG_ID"/> + <index-column name="DATETO"/> + </index> <index name="M_VALUED_STOCK_AGG_PERIOD" unique="false"> <index-column name="C_PERIOD_ID"/> </index> diff -r e349ce84e231 -r 205a911b298f src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct.html --- a/src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct.html Fri Mar 03 10:49:49 2017 +0100 +++ b/src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct.html Fri Mar 03 12:48:01 2017 +0100 @@ -13,7 +13,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2001-2011 Openbravo SLU + * All portions are Copyright (C) 2001-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -70,6 +70,30 @@ changeComboData(cmbWarehouse, arrWarehouse, cmbOrganization.options[cmbOrganization.selectedIndex].value, true); return true; } +function callbackCurrency(paramXMLParticular, XMLHttpRequestObj) { + var strText = ""; + if (getReadyStateHandler(XMLHttpRequestObj)) { + try { + if (XMLHttpRequestObj.responseText) { + strText = XMLHttpRequestObj.responseText; + if (strText && document.getElementById('inpCurrencyId').value != strText) { + document.getElementById('inpCurrencyId').value = strText; + refreshWarehouses(); + } + } + } catch (e) { + } + } + return true; +} +function setDefaultCurrency() { + try { + var paramXMLReq = null; + return submitXmlHttpRequest(callbackCurrency, document.frmMain, "CURRENCY", "ReportParetoProduct.html", false, null, paramXMLReq); + } catch (e) { + alert(e); + } +} </script> <script language="JavaScript" type="text/javascript" id="paramWarehouseArray"> @@ -260,7 +284,7 @@ <tr> <td class="TitleCell"><span class="LabelText">Organization</span></td> <td class="Combo_ContentCell" colspan="2"> - <select name="inpadOrgId" id="inpadOrgId" class="ComboKey Combo_TwoCells_width" onchange="refreshWarehouses();logChanges(this);return true;" required="true"> + <select name="inpadOrgId" id="inpadOrgId" class="ComboKey Combo_TwoCells_width" onchange="refreshWarehouses();setDefaultCurrency();logChanges(this);return true;" required="true"> <option value=""> <div id="reportAD_Org_ID"></div></option> </select> </td> diff -r e349ce84e231 -r 205a911b298f src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct.java --- a/src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct.java Fri Mar 03 10:49:49 2017 +0100 +++ b/src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct.java Fri Mar 03 12:48:01 2017 +0100 @@ -11,7 +11,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2001-2015 Openbravo SLU + * All portions are Copyright (C) 2001-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -21,24 +21,38 @@ import java.io.IOException; import java.io.PrintWriter; import java.math.BigDecimal; +import java.text.DateFormat; +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.Date; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.StringUtils; +import org.openbravo.base.exception.OBException; +import org.openbravo.base.filter.IsIDFilter; import org.openbravo.base.secureApp.HttpSecureAppServlet; import org.openbravo.base.secureApp.VariablesSecureApp; import org.openbravo.costing.CostingStatus; +import org.openbravo.costing.CostingUtils; +import org.openbravo.dal.core.OBContext; +import org.openbravo.dal.service.OBDal; import org.openbravo.erpCommon.businessUtility.WindowTabs; import org.openbravo.erpCommon.reference.PInstanceProcessData; import org.openbravo.erpCommon.utility.ComboTableData; import org.openbravo.erpCommon.utility.LeftTabsBar; import org.openbravo.erpCommon.utility.NavigationBar; +import org.openbravo.erpCommon.utility.OBCurrencyUtils; +import org.openbravo.erpCommon.utility.OBDateUtils; import org.openbravo.erpCommon.utility.OBError; import org.openbravo.erpCommon.utility.OBMessageUtils; import org.openbravo.erpCommon.utility.SequenceIdData; import org.openbravo.erpCommon.utility.ToolBar; import org.openbravo.erpCommon.utility.Utility; +import org.openbravo.model.common.enterprise.Organization; +import org.openbravo.model.materialmgmt.cost.CostingRule; import org.openbravo.xmlEngine.XmlDocument; public class ReportParetoProduct extends HttpSecureAppServlet { @@ -56,8 +70,11 @@ String strClient = vars.getClient(); String strAD_Org_ID = vars.getGlobalVariable("inpadOrgId", "ReportParetoProduct|AD_Org_ID", ""); - String strCurrencyId = vars.getGlobalVariable("inpCurrencyId", - "ReportParetoProduct|currency", strUserCurrencyId); + String strCurrencyId = OBCurrencyUtils.getOrgCurrency(strAD_Org_ID); + if (StringUtils.isEmpty(strCurrencyId)) { + strCurrencyId = strUserCurrencyId; + } + printPageDataSheet(request, response, vars, strWarehouse, strAD_Org_ID, strClient, strCurrencyId); } else if (vars.commandIn("FIND")) { @@ -85,6 +102,17 @@ "ReportParetoProduct|currency", strUserCurrencyId); printPageDataSheet(request, response, vars, strWarehouse, strAD_Org_ID, strClient, strCurrencyId); + } else if (vars.commandIn("CURRENCY")) { + String strOrg = vars.getRequestGlobalVariable("inpadOrgId", "ReportParetoProduct|AD_Org_ID", + IsIDFilter.instance); + if (StringUtils.isEmpty(strOrg)) { + strOrg = vars.getOrg(); + } + String strCurrencyId = OBCurrencyUtils.getOrgCurrency(strOrg); + response.setContentType("text/html; charset=UTF-8"); + PrintWriter out = response.getWriter(); + out.print(strCurrencyId); + out.close(); } else pageError(response); } @@ -116,10 +144,43 @@ OBError myMessage = null; myMessage = new OBError(); try { - data = ReportParetoProductData.select(this, strWarehouse, strClient, vars.getLanguage(), - strCurrencyId, strAD_Org_ID); + OBContext.setAdminMode(true); + + // Get legal entity (for aggregated table and currency conversion. The latter is only + // possible because the report can be launched only for one legal entity at the same time) + final Organization legalEntity = OBContext.getOBContext() + .getOrganizationStructureProvider(strClient) + .getLegalEntity(OBDal.getInstance().get(Organization.class, strAD_Org_ID)); + if (legalEntity == null) { + throw new OBException(OBMessageUtils.messageBD("WarehouseNotInLE")); + } + + // Calculate max aggregated date or set a default value if not aggregated data + String strMaxAggDate = ReportParetoProductData.selectMaxAggregatedDate(this, + legalEntity.getId()); + if (StringUtils.isBlank(strMaxAggDate)) { + final DateFormat formatter = new SimpleDateFormat("dd-MM-yyyy"); + final Date maxAggDate = formatter.parse("01-01-0000"); + strMaxAggDate = OBDateUtils.formatDate(maxAggDate); + } + + // Process Time (useful to avoid taking into account transactions for legacy costing rules) + String processTime = "01-01-1970 00:00:00"; + final CostingRule costRule = ReportValuationStock.getLEsCostingAlgortithm(legalEntity); + if (costRule != null) { + processTime = OBDateUtils.formatDate(CostingUtils.getCostingRuleStartingDate(costRule), + "dd-MM-yyyy HH:mm:ss"); + } + final String processTimeDateFormat = "DD-MM-YYYY HH24:MI:SS"; + + data = ReportParetoProductData.select(this, strCurrencyId, strClient, legalEntity.getId(), + processTime, processTimeDateFormat, strMaxAggDate, strWarehouse, strAD_Org_ID, + vars.getLanguage()); } catch (ServletException ex) { myMessage = Utility.translateError(this, vars, vars.getLanguage(), ex.getMessage()); + } catch (ParseException ignore) { + } finally { + OBContext.restorePreviousMode(); } strConvRateErrorMsg = myMessage.getMessage(); // If a conversion rate is missing for a certain transaction, an @@ -287,4 +348,5 @@ public String getServletInfo() { return "Servlet ReportParetoProduct info. Insert here any relevant information"; } // end of getServletInfo() method + } diff -r e349ce84e231 -r 205a911b298f src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct_data.xsql --- a/src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct_data.xsql Fri Mar 03 10:49:49 2017 +0100 +++ b/src/org/openbravo/erpCommon/ad_reports/ReportParetoProduct_data.xsql Fri Mar 03 12:48:01 2017 +0100 @@ -11,7 +11,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2001-2012 Openbravo SLU + * All portions are Copyright (C) 2001-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -24,75 +24,142 @@ <SqlClass name="ReportParetoProductData" package="org.openbravo.erpCommon.ad_reports"> <SqlClassComment></SqlClassComment> <SqlMethod name="select" type="preparedStatement" return="multiple"> - <SqlMethodComment></SqlMethodComment> + <SqlMethodComment>Five CTEs have been declared: + * org_prod_nonagg: this calculates the Value per warehouse's organization and product (it transforms each transaction cost to the given currency), + and it calculates the sum of movement quantities per warehouse's organization and product using the M_Transaction and M_Transaction_Cost tables directly. + It takes into account transactions just after the last costing rule available for the legal entity was started, and transactions and transaction costs non-aggregated yet + (if aggregation is not ready, it takes all the information from the M_Transaction and M_Transaction_Cost tables). + * org_prod_agg: It takes all the information from the M_Valued_Stock_Agg table (if available). Note this table aggregates information per legal entity, so if the Pareto is launched for + a non-legal entity organization, this CTE will return no records. If the report's currency is different from the aggregated currency a conversion is performed. + * org_prod: Creates the union all between org_prod_nonagg and org_prod_agg, grouping both datasets. Note that at this point the valuation of both datasets is in the same currency + * org_prod_perc: based on org_prod, calculates the cost as (the Value per org and product calculated on org_prod) / (sum of movement quantities calculated on org_prod). This could have been calculated in org_prod cte directly, but it's done here for clarity. + It also calculates the percentage per warehouse's organization and product as 100 * (the Value per record calculated on org_prod) / (total Value per warehouse's org). + * org_prod_perc_accum: based on org_prod_perc, calculates the percentage accumulated so far per warehouse's organization order by percentage desc. The accumulated percentages will be used to set ABC later on. + + The query outside the CTEs just gets the information from org_prod_perc_accum and calculates the ABC value based on the accumulated percentage, and joins with other tables to get the data to be printed. + </SqlMethodComment> <Sql><