Fixed bug 29664: Create Price List performance
authorVíctor Martínez Romanos <victor.martinez@openbravo.com>
Wed, 20 May 2015 16:30:14 +0200
changeset 26860 0b60872f78f5
parent 26859 7eae99f7fe79
child 26861 72a46a4bb692
Fixed bug 29664: Create Price List performance

The patch contains the following changes that improve the process performance (in my local environment the performance is increased an average of 80%):
1. count(*) inside M_GET_NO_TRX_PRODUCT_COST has been rewritten to use exists clause
2. Added 2 indices to M_Product_PO table on M_PRODUCT_ID and C_BPARTNER_ID columns to reduce seq. scans on this table
3. M_PRICELIST_CREATE: Force to analyze C_TEMP_Selection table in Postgres
According to PostgreSQL documentation, the autovacuum daemon cannot access and therefore cannot vacuum or analyze temporary tables. So it's recommended to run ANALYZE after the table is populated to improve performance.
Oracle doesn't seem to be affected by this problem, so the analyze is only executed for postgres
4. M_PRICELIST_CREATE: Complex queries that fill the C_TEMP_Selection has been rewritten using a dynamic SQL run through the EXECUTE command.
Although these queries are in general fast, they are executed as many times as records to be inserted into the Price List. So, the bigger amount of records the slower the process is.
It has been detected that the execution plan created for these queries can be improved a lot if we remove useless parts in the where clause (OR stuff). That's why we dynamically build the sql based on the parameters, which drastically improves the execution plan performance.
5. M_PRICELIST_CREATE: Removed useless join to ad_client table to calculate the client's currency. Instead we get it at the beginning of the process just one time.
6. M_PRICELIST_CREATE: Removed code included into the IF (v_Costbased = 'N'AND (v_PriceList_Version_Base_ID IS NULL)). This code is never executed, because there is a validation at the beginning of the process to avoid that situation.

Other changes not included that could improve performance:
1. Changing the db functions ad_isorgincluded and ad_org_isinnaturaltree from VOLATILE to STABLE reduces execution time for cost based price lists.
Right now the DBSM doesn't support this flag, so a feature request #29943 has been created
src-db/database/model/functions/M_GET_NO_TRX_PRODUCT_COST.xml
src-db/database/model/functions/M_PRICELIST_CREATE.xml
src-db/database/model/tables/M_PRODUCT_PO.xml
--- a/src-db/database/model/functions/M_GET_NO_TRX_PRODUCT_COST.xml	Wed May 27 01:29:05 2015 +0530
+++ b/src-db/database/model/functions/M_GET_NO_TRX_PRODUCT_COST.xml	Wed May 20 16:30:14 2015 +0200
@@ -34,7 +34,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) 2012 Openbravo SLU
+* All portions are Copyright (C) 2012-2015 Openbravo SLU
 * All Rights Reserved.
 * Contributor(s):  ______________________________________.
 ************************************************************************/
@@ -68,16 +68,20 @@
     END IF;
   END IF;
 
-  SELECT COUNT(*) INTO v_Count
-  FROM m_costing
-  WHERE datefrom <= p_movementdate
-    AND dateto > p_movementdate
-    AND m_product_id = p_product_id
-    AND COALESCE(m_warehouse_id, p_warehouse_id, '-1') = COALESCE(p_warehouse_id, m_warehouse_id, '-1')
-    AND ad_org_id = v_legal_entity
-    AND cost IS NOT NULL
-    AND costtype IN ('STA', 'AVA')
-    AND costtype = COALESCE(v_CostType, costtype);
+
+  SELECT count(*) INTO v_Count
+  FROM DUAL 
+  WHERE EXISTS (SELECT 1
+                FROM m_costing
+                WHERE datefrom <= p_movementdate
+                  AND dateto > p_movementdate
+                  AND m_product_id = p_product_id
+                  AND COALESCE(m_warehouse_id, p_warehouse_id, '-1') = COALESCE(p_warehouse_id, m_warehouse_id, '-1')
+                  AND ad_org_id = v_legal_entity
+                  AND cost IS NOT NULL
+                  AND costtype IN ('STA', 'AVA')
+                  AND costtype = COALESCE(v_CostType, costtype)
+                );
   IF(v_Count = 0) THEN
     RETURN NULL;
   END IF;
--- a/src-db/database/model/functions/M_PRICELIST_CREATE.xml	Wed May 27 01:29:05 2015 +0530
+++ b/src-db/database/model/functions/M_PRICELIST_CREATE.xml	Wed May 20 16:30:14 2015 +0200
@@ -52,6 +52,7 @@
     v_Costbased M_PriceList.Costbased%TYPE;
     v_validfromdate M_PriceList_Version.ValidFrom%TYPE;
     v_isCostMigrated NUMBER;
+    v_clientCurrencyId AD_Client.C_Currency_ID%TYPE;
     --
     -- Get PL Parameter
     Cur_DiscountLine RECORD;
@@ -62,6 +63,10 @@
     v_user AD_USER.AD_USER_ID%TYPE;
     
     v_shline_count rowcount%TYPE;
+
+    v_Sql_analyze_pg VARCHAR2(2000):='ANALYZE C_TEMP_Selection';
+    v_Sql_insert VARCHAR2(2000);
+    v_rdbms VARCHAR2(2000):=AD_GET_RDBMS();
     
   BEGIN
     --  Update AD_PInstance
@@ -100,12 +105,31 @@
     FROM DUAL
     WHERE EXISTS (SELECT 1 FROM ad_preference
                   WHERE attribute = 'Cost_Eng_Ins_Migrated');
-    SELECT M_PriceList_Version_Base_ID , Costbased, validfrom
-    INTO v_PriceList_Version_Base_ID, v_Costbased, v_validfromdate
+    -- Get PriceList Info
+    v_ResultStr:='GetPLInfo';
+    DBMS_OUTPUT.PUT_LINE(v_ResultStr) ;
+    SELECT p.C_Currency_ID,
+      c.priceprecision,
+      v.AD_Client_ID,
+      v.AD_Org_ID,
+      v.UpdatedBy,
+      v.M_DiscountSchema_ID,
+      M_PriceList_Version_Base_ID, Costbased, validfrom, cl.c_currency_id
+    INTO v_Currency_ID,
+      v_StdPrecision,
+      v_Client_ID,
+      v_Org_ID,
+      v_UpdatedBy,
+      v_DiscountSchema_ID,
+      v_PriceList_Version_Base_ID, v_Costbased, v_validfromdate, v_clientCurrencyId
     FROM M_PriceList p,
-         M_PriceList_Version v
+      M_PriceList_Version v,
+      C_Currency c,
+      AD_Client cl
     WHERE p.M_PriceList_ID=v.M_PriceList_ID
-        AND v.M_PriceList_Version_ID=v_PriceList_Version_ID;  
+      AND p.C_Currency_ID=c.C_Currency_ID
+      AND cl.ad_client_id = p.ad_client_id
+      AND v.M_PriceList_Version_ID=v_PriceList_Version_ID;
     IF (v_Costbased = 'N' AND (v_PriceList_Version_Base_ID IS NULL OR v_PriceList_Version_Base_ID='')) THEN
       RAISE_APPLICATION_ERROR(-20000, '@BasePriceListRequired@');
     END IF;
@@ -201,29 +225,6 @@
         v_Message:='@Deleted@=' || rowcount || ' - ';
         DBMS_OUTPUT.PUT_LINE(v_Message) ;
       END IF;
-      -- Get PriceList Info
-      v_ResultStr:='GetPLInfo';
-      DBMS_OUTPUT.PUT_LINE(v_ResultStr) ;
-      SELECT p.C_Currency_ID,
-        c.priceprecision,
-        v.AD_Client_ID,
-        v.AD_Org_ID,
-        v.UpdatedBy,
-        v.M_DiscountSchema_ID,
-        M_PriceList_Version_Base_ID
-      INTO v_Currency_ID,
-        v_StdPrecision,
-        v_Client_ID,
-        v_Org_ID,
-        v_UpdatedBy,
-        v_DiscountSchema_ID,
-        v_PriceList_Version_Base_ID
-      FROM M_PriceList p,
-        M_PriceList_Version v,
-        C_Currency c
-      WHERE p.M_PriceList_ID=v.M_PriceList_ID
-        AND p.C_Currency_ID=c.C_Currency_ID
-        AND v.M_PriceList_Version_ID=v_PriceList_Version_ID;
       /**
       * For All Discount Lines in Sequence
       */
@@ -243,80 +244,90 @@
         -- -----------------------------------
         -- Create Selection in temporary table
         -- -----------------------------------
-        IF(v_Costbased = 'N'AND (v_PriceList_Version_Base_ID IS NULL)) THEN
-          -- Create Selection from M_Product_PO
-          INSERT
-          INTO C_TEMP_Selection
-            (
-              C_TEMP_Selection_ID
-            )
-          SELECT DISTINCT po.M_Product_ID
-          FROM M_Product p,
-            M_Product_PO po
-          WHERE p.M_Product_ID=po.M_Product_ID
-            AND(p.AD_Client_ID=v_Client_ID
-            OR p.AD_Client_ID='0')
-            AND p.IsActive='Y'
-            AND po.IsActive='Y'
-            AND po.IsCurrentVendor='Y'  -- Optional Restrictions
-            AND(Cur_DiscountLine.M_Product_Category_ID IS NULL
-            OR p.M_Product_Category_ID=Cur_DiscountLine.M_Product_Category_ID)
-            AND(Cur_DiscountLine.C_BPartner_ID IS NULL
-            OR po.C_BPartner_ID=Cur_DiscountLine.C_BPartner_ID)
-            AND(Cur_DiscountLine.M_Product_ID IS NULL
-            OR p.M_Product_ID=Cur_DiscountLine.M_Product_ID) ;
-            
-        ELSIF (v_Costbased = 'Y' AND (v_PriceList_Version_Base_ID IS NULL OR v_PriceList_Version_Base_ID='')) THEN    
-          INSERT
-          INTO C_TEMP_Selection
-            (
-              C_TEMP_Selection_ID
-            )
-           SELECT DISTINCT p.M_Product_id
-           FROM M_Product p
-           inner join M_costing co on p.M_Product_ID=co.M_Product_ID
-           WHERE (p.AD_Client_ID=v_Client_ID
-           OR p.AD_Client_ID='0')
-           AND p.IsActive='Y'
-           AND AD_ORG_ISINNATURALTREE(v_org_id,co.ad_org_id,v_Client_ID)='Y'
-           AND(Cur_DiscountLine.M_Product_Category_ID IS NULL
-           OR p.M_Product_Category_ID=Cur_DiscountLine.M_Product_Category_ID)
-           AND(Cur_DiscountLine.C_BPartner_ID IS NULL
-           OR p.C_BPartner_ID=Cur_DiscountLine.C_BPartner_ID)
-           AND(Cur_DiscountLine.M_Product_ID IS NULL
-           OR p.M_Product_ID=Cur_DiscountLine.M_Product_ID)
-           AND TRUNC(datefrom)<=v_validfromdate AND TRUNC(dateto)>v_validfromdate
-           AND NOT EXISTS
-           (select M_Product_id from c_discount d where co.M_Product_id=d.M_Product_id);
+        IF (v_Costbased = 'Y' AND (v_PriceList_Version_Base_ID IS NULL OR v_PriceList_Version_Base_ID='')) THEN    
+          v_Sql_insert := '
+                  INSERT
+                  INTO C_TEMP_Selection
+                    (
+                      C_TEMP_Selection_ID
+                    )
+                   SELECT DISTINCT p.M_Product_id
+                   FROM M_Product p
+                   inner join M_costing co on p.M_Product_ID=co.M_Product_ID
+                   WHERE p.AD_Client_ID in (''0'', ''' || v_Client_ID || ''')
+                   AND p.IsActive=''Y''
+                   AND AD_ORG_ISINNATURALTREE(''' || v_org_id || ''',co.ad_org_id,''' || v_Client_ID || ''')=''Y'' 
+                   AND NOT EXISTS (select 1 from c_discount d where co.M_Product_id=d.M_Product_id) ';
+          
+          IF (v_rdbms = 'POSTGRE') THEN
+            v_Sql_insert := v_Sql_insert || '
+                   AND TRUNC(datefrom)<= $1 AND TRUNC(dateto)> $2 ';
+          ELSE
+            v_Sql_insert := v_Sql_insert || '
+                   AND TRUNC(datefrom)<= :1 AND TRUNC(dateto)> :2 ';
+          END IF;
+          IF (Cur_DiscountLine.M_Product_Category_ID IS NOT NULL) THEN
+            v_Sql_insert := v_Sql_insert || ' 
+                   AND p.M_Product_Category_ID=''' || Cur_DiscountLine.M_Product_Category_ID || ''' ';
+          END IF;
+          IF (Cur_DiscountLine.C_BPartner_ID IS NOT NULL) THEN
+            v_Sql_insert := v_Sql_insert || ' 
+                   AND p.C_BPartner_ID=''' || Cur_DiscountLine.C_BPartner_ID || ''' ';
+          END IF;
+          IF (Cur_DiscountLine.M_Product_ID IS NOT NULL) THEN
+            v_Sql_insert := v_Sql_insert || ' 
+                   AND p.M_Product_ID= ''' || Cur_DiscountLine.M_Product_ID || ''' ';
+          END IF;
+          -- Force the execution this way to improve query performance
+          EXECUTE IMMEDIATE v_Sql_insert USING v_validfromdate, v_validfromdate;
+          
         ELSE
           -- Create Selection from existing PriceList
-          INSERT
-          INTO C_TEMP_Selection
-            (
-              C_TEMP_Selection_ID
-            )
-          SELECT DISTINCT p.M_Product_ID
-          FROM M_Product p,
-            M_ProductPrice pp
-          WHERE p.M_Product_ID=pp.M_Product_ID
-            AND pp.M_PriceList_Version_ID=v_PriceList_Version_Base_ID
-            AND p.IsActive='Y'
-            AND pp.IsActive='Y'  -- Optional Restrictions
-            AND(Cur_DiscountLine.M_Product_Category_ID IS NULL
-            OR p.M_Product_Category_ID=Cur_DiscountLine.M_Product_Category_ID)
-            AND(Cur_DiscountLine.C_BPartner_ID IS NULL
-            OR EXISTS
-            (SELECT *
-            FROM M_Product_PO po
-            WHERE po.M_Product_ID=p.M_Product_ID
-              AND po.C_BPartner_ID=Cur_DiscountLine.C_BPartner_ID
-            ))
-            AND(Cur_DiscountLine.M_Product_ID IS NULL
-            OR p.M_Product_ID=Cur_DiscountLine.M_Product_ID) ;
+          v_Sql_insert := '
+                  INSERT
+                  INTO C_TEMP_Selection
+                    (
+                      C_TEMP_Selection_ID
+                    )
+
+                  SELECT p.M_Product_ID
+                  FROM M_Product p,
+                    M_ProductPrice pp
+                  WHERE p.M_Product_ID=pp.M_Product_ID
+                    AND pp.M_PriceList_Version_ID= ''' || v_PriceList_Version_Base_ID || '''
+                    AND p.IsActive=''Y''
+                    AND pp.IsActive=''Y''  ';
+
+          IF (Cur_DiscountLine.M_Product_Category_ID IS NOT NULL) THEN
+            v_Sql_insert := v_Sql_insert || ' 
+                   AND p.M_Product_Category_ID=''' || Cur_DiscountLine.M_Product_Category_ID || ''' ';
+          END IF;
+          IF (Cur_DiscountLine.C_BPartner_ID IS NOT NULL) THEN
+            v_Sql_insert := v_Sql_insert || ' 
+                   AND EXISTS
+	                    (SELECT 1
+	                    FROM M_Product_PO po
+	                    WHERE po.M_Product_ID=p.M_Product_ID
+	                      AND po.C_BPartner_ID=''' || Cur_DiscountLine.C_BPartner_ID ||''') ';
+          END IF;
+          IF (Cur_DiscountLine.M_Product_ID IS NOT NULL) THEN
+            v_Sql_insert := v_Sql_insert || ' 
+                   AND p.M_Product_ID= ''' || Cur_DiscountLine.M_Product_ID || ''' ';
+          END IF;
+          -- Force the execution this way to improve query performance
+          EXECUTE IMMEDIATE v_Sql_insert;
+
         END IF;
+        
         rowcount:=SQL%ROWCOUNT;
         v_Message:=v_Message || '@Selected@=' || rowcount;
         -- DBMS_OUTPUT.PUT_LINE(v_Message);
+
+        -- Temporary tables are not accessed by the autovacuum daemon, so we force an analyze to calculate index
+        IF (v_rdbms = 'POSTGRE') THEN
+          EXECUTE IMMEDIATE v_Sql_analyze_pg;
+        END IF;
+
         -- Delete Prices in Selection, so that we can insert
         IF(v_PriceList_Version_Base_ID IS NULL  OR v_PriceList_Version_Base_ID<>v_PriceList_Version_ID) THEN
           v_ResultStr:=v_ResultStr || ', Delete';
@@ -324,7 +335,7 @@
           FROM M_ProductPrice
           WHERE M_ProductPrice.M_PriceList_Version_ID=v_PriceList_Version_ID
             AND EXISTS
-            (SELECT *
+            (SELECT 1
             FROM C_TEMP_Selection s
             WHERE M_ProductPrice.M_Product_ID=s.C_TEMP_Selection_ID
             )
@@ -362,7 +373,7 @@
             COALESCE(C_Currency_Convert(po.PricePO, po.C_Currency_ID, v_Currency_ID, Cur_DiscountLine.ConversionDate, Cur_DiscountLine.ConversionRateType, v_Client_ID, v_Org_ID), 0)
           FROM M_Product_PO po
           WHERE EXISTS
-            (SELECT * FROM C_TEMP_Selection s WHERE po.M_Product_ID=s.C_TEMP_Selection_ID)
+            (SELECT 1 FROM C_TEMP_Selection s WHERE po.M_Product_ID=s.C_TEMP_Selection_ID)
             AND po.IsCurrentVendor='Y'
             AND po.IsActive='Y';      
          ELSIF(v_costbased='Y')THEN   
@@ -378,19 +389,17 @@
               PriceLimit,
               Cost 
             )
-
             SELECT  get_uuid(), v_PriceList_Version_ID,
             mp.M_Product_ID, v_Client_ID, v_Org_ID, 'Y',
             now(), v_UpdatedBy, now(), v_UpdatedBy, 
-            COALESCE(M_GET_COST (mp.M_Product_ID, add_hms(v_validfromdate, 23, 59, 59), CASE v_isCostMigrated WHEN 0 THEN mp.costtype ELSE null END, v_Org_ID, mp.ad_client_id, null, ac.c_currency_id, v_Currency_ID),0), 
-            COALESCE(M_GET_COST (mp.M_Product_ID, add_hms(v_validfromdate, 23, 59, 59), CASE v_isCostMigrated WHEN 0 THEN mp.costtype ELSE null END, v_Org_ID, mp.ad_client_id, null, ac.c_currency_id, v_Currency_ID),0), 
-            COALESCE(M_GET_COST (mp.M_Product_ID, add_hms(v_validfromdate, 23, 59, 59), CASE v_isCostMigrated WHEN 0 THEN mp.costtype ELSE null END, v_Org_ID, mp.ad_client_id, null, ac.c_currency_id, v_Currency_ID),0), 
-            COALESCE(M_GET_COST (mp.M_Product_ID, add_hms(v_validfromdate, 23, 59, 59), CASE v_isCostMigrated WHEN 0 THEN mp.costtype ELSE null END, v_Org_ID, mp.ad_client_id, null, ac.c_currency_id, v_Currency_ID),0)
-            from m_product mp, ad_client ac
-            where mp.ad_client_id=ac.ad_client_id
-            AND EXISTS
-            (SELECT 1 FROM C_TEMP_Selection s WHERE mp.M_Product_ID=s.C_TEMP_Selection_ID)
+            COALESCE(M_GET_COST (mp.M_Product_ID, add_hms(v_validfromdate, 23, 59, 59), CASE v_isCostMigrated WHEN 0 THEN mp.costtype ELSE null END, v_Org_ID, mp.ad_client_id, null, v_clientCurrencyId, v_Currency_ID),0),  
+            COALESCE(M_GET_COST (mp.M_Product_ID, add_hms(v_validfromdate, 23, 59, 59), CASE v_isCostMigrated WHEN 0 THEN mp.costtype ELSE null END, v_Org_ID, mp.ad_client_id, null, v_clientCurrencyId, v_Currency_ID),0), 
+            COALESCE(M_GET_COST (mp.M_Product_ID, add_hms(v_validfromdate, 23, 59, 59), CASE v_isCostMigrated WHEN 0 THEN mp.costtype ELSE null END, v_Org_ID, mp.ad_client_id, null, v_clientCurrencyId, v_Currency_ID),0), 
+            COALESCE(M_GET_COST (mp.M_Product_ID, add_hms(v_validfromdate, 23, 59, 59), CASE v_isCostMigrated WHEN 0 THEN mp.costtype ELSE null END, v_Org_ID, mp.ad_client_id, null, v_clientCurrencyId, v_Currency_ID),0)
+            from m_product mp
+            where EXISTS (SELECT 1 FROM C_TEMP_Selection s WHERE mp.M_Product_ID=s.C_TEMP_Selection_ID)
             AND mp.IsActive='Y'; 
+            
          ELSE
           -- Copy and Convert from other PriceList_Version
           v_ResultStr:=v_ResultStr || ',Copy_PL';
@@ -420,7 +429,7 @@
             ON(plv.M_PriceList_ID=pl.M_PriceList_ID)
           WHERE pp.M_PriceList_Version_ID=v_PriceList_Version_Base_ID
             AND EXISTS
-            (SELECT * FROM C_TEMP_Selection s WHERE pp.M_Product_ID=s.C_TEMP_Selection_ID)
+            (SELECT 1 FROM C_TEMP_Selection s WHERE pp.M_Product_ID=s.C_TEMP_Selection_ID)
             AND pp.IsActive='Y';
         END IF;
         rowcount:=SQL%ROWCOUNT;
@@ -444,7 +453,7 @@
           ) + Cur_DiscountLine.Limit_AddAmt) *(1 - Cur_DiscountLine.Limit_Discount/100)
         WHERE M_PriceList_Version_ID=v_PriceList_Version_ID
           AND EXISTS
-          (SELECT *
+          (SELECT 1
           FROM C_TEMP_Selection s
           WHERE s.C_TEMP_Selection_ID=M_ProductPrice.M_Product_ID
           )
@@ -494,7 +503,7 @@
           ) -- Currency
         WHERE M_PriceList_Version_ID=v_PriceList_Version_ID
           AND EXISTS
-          (SELECT *
+          (SELECT 1
           FROM C_TEMP_Selection s
           WHERE s.C_TEMP_Selection_ID=M_ProductPrice.M_Product_ID
           )
@@ -519,7 +528,7 @@
           updated=now()
         WHERE M_PriceList_Version_ID=v_PriceList_Version_ID
           AND EXISTS
-          (SELECT *
+          (SELECT 1
           FROM C_TEMP_Selection s
           WHERE s.C_TEMP_Selection_ID=M_ProductPrice.M_Product_ID
           )
--- a/src-db/database/model/tables/M_PRODUCT_PO.xml	Wed May 27 01:29:05 2015 +0530
+++ b/src-db/database/model/tables/M_PRODUCT_PO.xml	Wed May 20 16:30:14 2015 +0200
@@ -155,6 +155,12 @@
       <foreign-key foreignTable="AD_ORG" name="M_PRODUCTPO_ORG">
         <reference local="AD_ORG_ID" foreign="AD_ORG_ID"/>
       </foreign-key>
+      <index name="M_PROD_PO_BPARTNER_IDX" unique="false">
+        <index-column name="C_BPARTNER_ID"/>
+      </index>
+      <index name="M_PROD_PO_PRODUCT_IDX" unique="false">
+        <index-column name="M_PRODUCT_ID"/>
+      </index>
       <unique name="M_PRODUCT_PO_PRODUCT_BPARTN_UN">
         <unique-column name="M_PRODUCT_ID"/>
         <unique-column name="C_BPARTNER_ID"/>