openerp-community team mailing list archive

["Nhomar Hernandez \(Vauxoo\)" <nhomar@xxxxxxxxx>]

Nhomar Hernandez (Vauxoo) has proposed merging lp:~openerp-community/openobject-addons/trunk-btree_compute-vauxoo into lp:openobject-addons.

Requested reviews:
  Olivier Dony (OpenERP) (odo-openerp)
  OpenERP R&D Team (openerp-dev): finish, learn improve
Related bugs:
  Bug #887376 in OpenERP Addons: "[6.0 & 6.1] [account] def compute needs optimization"
  https://bugs.launchpad.net/openobject-addons/+bug/887376

For more details, see:
https://code.launchpad.net/~openerp-community/openobject-addons/trunk-btree_compute-vauxoo/+merge/127138

Account compute with SQL b-tree

Description:

With severals accounting entries the algorith to compute totals on accounting is so unefficient, It is happening because we are not using b-tree sql but all data on chart of account is ready to use with this advantage, some information about what b-tree is;:

http://www.simple-talk.com/sql/t-sql-programming/binary-trees-in-sql/

We have 3 big implementations with +1000 accounts and +2Millions of records, compute the tree view on chart of account was a big problem and the speed on hardware never was enought, for this reason we took the time of rewrite all compute method and prepare this merge proposal.

>From Vauxoo we are tottally sure we can not stay +1 year waiting for this improvement due to we are becoming the biggest open source ERP system and community.

Please test and give us feedback.

Some Important points:

We are proposing this MP with both methods and one variable to be able to use both, technically it not so ellegant but due to the impact that accounting can receive with this improvement we are being carefully, you can manage both methods with the same code, just uncomment one line and your code will be the original one.

Some comments on:

https://code.launchpad.net/~vauxoo/openobject-addons/trunk-bug_887376-account_move_compute_with_btree-moylop260/+merge/89623

But these is for work with testing YAML

FUNCTIONAL LIST:
  -Calculation field foreign currency

YAML TEST:
  -Draft are not considered in the sum.
  -Consolidated accounts.
  -2 Invoices.
  -Multicurrency invoices.
  -Draft and unbalanced entries.
  -Consolidated of different multi-company
  
Please Help us to develop several YAML tests to be sure everything will be fine with it!

Resubmited for internal Launchpad Error with diff! - I hope mantain this branch alive until everybody test it.

All around this improvement can impact A LOT the generation of Financial reports
-- 
https://code.launchpad.net/~openerp-community/openobject-addons/trunk-btree_compute-vauxoo/+merge/127138
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-addons/trunk-btree_compute-vauxoo.

=== modified file 'account/__openerp__.py'
--- account/__openerp__.py	2012-09-26 12:16:27 +0000
+++ account/__openerp__.py	2012-09-30 05:17:19 +0000
@@ -155,6 +155,7 @@
         #'test/account_cash_statement.yml',
         'test/test_edi_invoice.yml',
         'test/account_report.yml',
+        'test/account_compute.yml',
         'test/account_fiscalyear_close_state.yml', #last test, as it will definitively close the demo fiscalyear
     ],
     'installable': True,

=== modified file 'account/account.py'
--- account/account.py	2012-09-29 12:07:24 +0000
+++ account/account.py	2012-09-30 05:17:19 +0000
@@ -263,7 +263,7 @@
             ids3 = self._get_children_and_consol(cr, uid, ids3, context)
         return ids2 + ids3
 
-    def __compute(self, cr, uid, ids, field_names, arg=None, context=None,
+    def __compute_ORIGINAL(self, cr, uid, ids, field_names, arg=None, context=None,
                   query='', query_params=()):
         """ compute the balance, debit and/or credit for the provided
         account ids
@@ -359,7 +359,107 @@
             for id in ids:
                 res[id] = null_result
         return res
+    
+    def __compute_BTREE(self, cr, uid, ids, field_names, arg=None, context=None,
+                      query='', query_params=()):
+        """ compute the balance, debit and/or credit for the provided
+        account ids
 
+        Arguments:
+        `ids`: account ids
+        `field_names`: the fields to compute (a list of any of
+                       'balance', 'debit' and 'credit')
+        `arg`: unused fields.function stuff
+        `query`: additional query filter (as a string)
+        `query_params`: parameters for the provided query string
+                        (__compute will handle their escaping) as a
+                        tuple
+        """
+        mapping = {
+            'debit': "COALESCE(SUM(l.debit), 0) AS debit",
+            'credit': "COALESCE(SUM(l.credit), 0) AS credit",
+            # by convention, foreign_balance is 0 when the account has no secondary currency, because the amounts may be in different currencies
+            # Calculate is wrong with these new sql
+            #'foreign_balance': "(SELECT CASE WHEN currency_id IS NULL THEN 0 ELSE COALESCE(SUM(l.amount_currency), 0) END FROM account_account WHERE id IN (l.account_id)) as foreign_balance",
+            'foreign_balance': "CASE WHEN MIN(parent_currency_id) IS NULL THEN 0 ELSE COALESCE(SUM(l.amount_currency), 0) END AS foreign_balance"
+        }
+        mapping2 = {
+            'balance': "debit-credit AS balance"
+        }
+        
+        accounts = {}
+        aml_query = self.pool.get('account.move.line')._query_get(cr, uid, context=context)
+        
+        wheres = [""]
+        if query.strip():
+            wheres.append(query.strip())
+        if aml_query.strip():
+            wheres.append(aml_query.strip())
+        filters = " AND ".join(wheres)
+        self.logger.notifyChannel('addons.'+self._name, netsvc.LOG_DEBUG,
+                                  'Filters: %s'%filters)
+        #children_and_consolidated = self._get_children_and_consol(
+            #cr, uid, ids, context=context)
+        children_and_consolidated = ids #Now we don't need get children_and_consolidated from function. Now is from query b-tree 
+        if children_and_consolidated:
+            request= '''SELECT *, ''' +\
+                ', '.join( mapping2.values() ) +\
+            '''
+            FROM (
+                SELECT account_child_and_consolidated.parent_id AS id,''' +\
+                       ', '.join( mapping.values() ) +\
+            '''
+                FROM (
+                    SELECT account_child_vw.parent_id, COALESCE( account_consolidated_vw.child_id, account_child_vw.child_id) AS child_id
+                    , COALESCE( account_consolidated_vw.parent_currency_id, account_child_vw.parent_currency_id) AS parent_currency_id
+                    FROM (
+                        SELECT aa_tree_1.id AS parent_id, aa_tree_2.id AS child_id, aa_tree_1.currency_id AS parent_currency_id
+                        FROM account_account aa_tree_1
+                        LEFT OUTER JOIN account_account aa_tree_2
+                           ON aa_tree_2.parent_left 
+                              BETWEEN aa_tree_1.parent_left AND aa_tree_1.parent_right
+                    ) account_child_vw
+                    LEFT OUTER JOIN 
+                    (
+                        SELECT aa_tree_1.id AS parent_id, aa_tree_4.id AS child_id, aa_tree_1.currency_id AS parent_currency_id
+                        FROM account_account_consol_rel
+                        INNER JOIN account_account aa_tree_1
+                           ON aa_tree_1.id = account_account_consol_rel.child_id
+                        INNER JOIN account_account aa_tree_2
+                           ON aa_tree_2.id = account_account_consol_rel.parent_id
+                        LEFT OUTER JOIN account_account aa_tree_3
+                           ON aa_tree_3.parent_left 
+                              BETWEEN aa_tree_1.parent_left AND aa_tree_1.parent_right
+                        LEFT OUTER JOIN account_account aa_tree_4
+                           ON aa_tree_4.parent_left 
+                              BETWEEN aa_tree_2.parent_left AND aa_tree_2.parent_right
+                    ) account_consolidated_vw
+                    ON account_child_vw.child_id = account_consolidated_vw.parent_id
+                ) account_child_and_consolidated
+                LEFT OUTER JOIN account_move_line l
+                  ON l.account_id = account_child_and_consolidated.child_id
+                ''' \
+               'WHERE account_child_and_consolidated.parent_id IN %s ' \
+                + filters + \
+                'GROUP BY account_child_and_consolidated.parent_id ) subvw'
+            params = (tuple(children_and_consolidated),) + query_params
+            cr.execute(request, params)
+            self.logger.notifyChannel('addons.'+self._name, netsvc.LOG_DEBUG,
+                                      'Status: %s'%cr.statusmessage)
+            
+            for res1 in cr.dictfetchall():
+                accounts[res1['id']] = res1
+        #TODO: Process multi-currency
+        res = {}
+        for id in ids:
+            for field_name in field_names:
+                res.setdefault(id, {})[field_name] = accounts.get(id, {}).get(field_name, 0.0)
+        return res
+    
+    ##Switch function for test
+    #__compute = __compute_ORIGINAL
+    __compute = __compute_BTREE
+    
     def _get_company_currency(self, cr, uid, ids, field_name, arg, context=None):
         result = {}
         for rec in self.browse(cr, uid, ids, context=context):

=== added file 'account/test/account_compute.yml'
--- account/test/account_compute.yml	1970-01-01 00:00:00 +0000
+++ account/test/account_compute.yml	2012-09-30 05:17:19 +0000
@@ -0,0 +1,111 @@
+-
+  Verify account compute of supplier invoice
+-
+  !python {model: account.account, id: account_account_expense_consolidated0}:
+    # Temporarily stop deferring parent_store computation
+    self.pool._init = False
+-
+  I verify that amount of account expense is calculated correctly.
+-
+  !assert {model: account.account, id: account.a_expense, string: Amount of account expense is not calculated correctly.}:
+    - int( debit ) == 3100 and int( credit ) == 0 and int( balance ) == 3100
+-
+  I verify that amount of account parent expense is calculated correctly.
+-   
+  !assert {model: account.account, id: account.a_expense, string: Amount of account parent expense is not calculated correctly.}:
+    - int( parent_id.debit ) == 3100 and int( parent_id.credit ) == 0 and int( parent_id.balance ) == 3100
+-
+  I create a consolidated expense account
+-
+  !record {model: account.account, id: account_account_expense_consolidated0}:
+    code: 90001_test_consolidated_expense
+    company_id: base.main_company
+    currency_mode: current
+    name: Consolidated Expense For Test
+    type: consolidation
+    user_type: account.data_account_type_view
+    child_consol_ids:
+        - account.a_expense
+    parent_id: account.chart0
+-
+  I verify that debit amount of account consolidated expense is calculated correctly.
+-
+  !assert {model: account.account, id: account_account_expense_consolidated0, string: Amount of account consolidated expense is not calculated correctly.}:
+    - int( debit ) == 3100 and int( credit ) == 0 and int( balance ) == 3100
+
+#Verify account compute of customer invoice
+-
+  I verify that amount of account income is calculated correctly.
+-
+  !assert {model: account.account, id: account.a_sale, string: Amount of account income is not calculated correctly.}:
+    - int( debit ) == 0 and int( credit ) == 9000 and int( balance ) == -9000
+-
+  I verify that amount of account parent income is calculated correctly.
+-   
+  !assert {model: account.account, id: account.a_sale, string: Amount of account parent income is not calculated correctly.}:
+    - int( parent_id.debit ) == 0 and int( parent_id.credit ) == 9000 and int( parent_id.balance ) == -9000
+-
+  I create a consolidated income account
+-
+  !record {model: account.account, id: account_account_income_consolidated0}:
+    code: 90002_test_consolidated_income
+    company_id: base.main_company
+    currency_mode: current
+    name: Consolidated income For Test
+    type: consolidation
+    user_type: account.data_account_type_view
+    child_consol_ids:
+        - account.a_sale
+    parent_id: account.chart0
+-
+  I verify that debit amount of account consolidated income is calculated correctly.
+-
+  !assert {model: account.account, id: account_account_income_consolidated0, string: Amount of account consolidated income is not calculated correctly.}:
+    - int( debit ) == 0 and int( credit ) == 9000 and int( balance ) == -9000
+-
+  I create a account Debtor empty
+-
+  !record {model: account.account, id: account_account_debtor_2}:
+    code: 90003_test_debtor2
+    company_id: base.main_company
+    currency_mode: current
+    name: Debtor2
+    type: receivable
+    user_type: account.data_account_type_receivable
+    parent_id: account.chart0
+    currency_id: base.USD
+-
+  I assign manually new account to invoice currency
+-
+  !python {model: account.invoice}: |
+    writed = self.write(cr, uid, ref('account_invoice_currency'), {
+        'account_id': ref('account_account_debtor_2'),
+    })
+    assert writed, "Account has not been assigned correctly"  
+
+-
+  I validate invoice currency by clicking on Create button
+-
+  !workflow {model: account.invoice, action: invoice_open, ref: account_invoice_currency}
+-
+  I check that the invoice state is "Open"
+-
+  !assert {model: account.invoice, id: account_invoice_currency}:
+    - state == 'open'
+-
+  I check that now there is a move attached to the invoice
+-
+  !python {model: account.invoice}: |
+    acc_id=self.browse(cr, uid, ref("account_invoice_currency"))
+    assert acc_id.move_id, "Move not created for open invoice"
+-
+  I verify with multi-currency compute amounts
+-
+  !assert {model: account.account, id: account_account_debtor_2, string: Wrong multi-currency compute amounts}:
+    - int( debit ) == 450 and int( credit ) == 0 and int( balance ) == 450
+
+-
+  !python {model: account.account, id: account_account_expense_consolidated0}:
+    # Restore deferring parent_store computation
+    self.pool._init = True
+