Merge lp:~openerp-community/openobject-addons/trunk-btree_compute-vauxoo into lp:openobject-addons

Proposed by Nhomar - Vauxoo on 2012-09-30
Status: Needs review
Proposed branch: lp:~openerp-community/openobject-addons/trunk-btree_compute-vauxoo
Merge into: lp:openobject-addons
Diff against target: 246 lines (+211/-1)
3 files modified
account/__openerp__.py (+1/-0)
account/account.py (+99/-1)
account/test/account_compute.yml (+111/-0)
To merge this branch: bzr merge lp:~openerp-community/openobject-addons/trunk-btree_compute-vauxoo
Reviewer Review Type Date Requested Status
Moisés López - http://www.vauxoo.com 2012-09-30 Pending
Fabien (Open ERP) 2012-09-30 Pending
Olivier Dony (Odoo) 2012-09-30 Pending
OpenERP R&D Team finish, learn improve 2012-09-30 Pending
Review via email: mp+127138@code.launchpad.net

Description of the change

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

To post a comment you must log in.
ali-mis (ahmadi-mis) wrote :
Download full text (3.3 KiB)

-------------------------------------------
Regards,
Ali AL-Ahmadi
+966597057057

To: <email address hidden>; <email address hidden>; <email address hidden>
From: <email address hidden>
Date: Sun, 30 Sep 2012 05:19:31 +0000
Subject: [Openerp-community] [Merge] lp:~openerp-community/openobject-addons/trunk-btree_compute-vauxoo into lp:openobject-addons

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.

_______________________________________________ Mailing l...

Read more...

Nhomar - Vauxoo (nhomar) wrote :

HEllo Ferdinand.

Thanks a LOt for your feedback, I remember the first partner day we see in
your machinde this module working,.

BUt in this case, is a diferent layer, complementary, yes, but different,
Here I m talking about change the SQL - technique to __compute, not the
computing it self.

I think combined, your option and our are complementary, not excluyent, and
very good.

I will test your module to be sure about what im talking, and will give
feedbak,

Question?: It works for V6.1?
2012/9/30 ferdinand <email address hidden>

> Hello!
>
> In addition to my previous mail it is obvious that all period based
> financial reports will experience a dramatic performance increase if
> based on period sums
>
> regards
> ferdinand
>

--
--------------------
Saludos Cordiales

Nhomar G. Hernandez M.
+58-414-4110269
Skype: nhomar00
Web-Blog: http://geronimo.com.ve
Servicios IT: http://vauxoo.com
Linux-Counter: 467724
Correos:
<email address hidden>
<email address hidden>
twitter @nhomar

7662. By Nhomar - Vauxoo on 2012-09-30

[FIX] Old logger failing, new logger now

7663. By Nhomar - Vauxoo on 2012-09-30

[MERGE] From Trunk to avoid failing the merge in future

Unmerged revisions

7663. By Nhomar - Vauxoo on 2012-09-30

[MERGE] From Trunk to avoid failing the merge in future

7662. By Nhomar - Vauxoo on 2012-09-30

[FIX] Old logger failing, new logger now

7661. By Nhomar - Vauxoo on 2012-09-30

[FIX] On comment in yaml

7660. By Moisés López - http://www.vauxoo.com on 2012-09-29

[ADD] All compute tests

7659. By Moisés López - http://www.vauxoo.com on 2012-09-29

[REF][IMP] Remaking all the algorithm to be able to compute
amount total in accounting with btree.
made by <email address hidden> branch re-pushed for big difference with trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account/__openerp__.py'
2--- account/__openerp__.py 2012-09-26 12:16:27 +0000
3+++ account/__openerp__.py 2012-09-30 22:10:28 +0000
4@@ -155,6 +155,7 @@
5 #'test/account_cash_statement.yml',
6 'test/test_edi_invoice.yml',
7 'test/account_report.yml',
8+ 'test/account_compute.yml',
9 'test/account_fiscalyear_close_state.yml', #last test, as it will definitively close the demo fiscalyear
10 ],
11 'installable': True,
12
13=== modified file 'account/account.py'
14--- account/account.py 2012-09-30 12:46:54 +0000
15+++ account/account.py 2012-09-30 22:10:28 +0000
16@@ -263,7 +263,7 @@
17 ids3 = self._get_children_and_consol(cr, uid, ids3, context)
18 return ids2 + ids3
19
20- def __compute(self, cr, uid, ids, field_names, arg=None, context=None,
21+ def __compute_ORIGINAL(self, cr, uid, ids, field_names, arg=None, context=None,
22 query='', query_params=()):
23 """ compute the balance, debit and/or credit for the provided
24 account ids
25@@ -359,7 +359,105 @@
26 for id in ids:
27 res[id] = null_result
28 return res
29+
30+ def __compute_BTREE(self, cr, uid, ids, field_names, arg=None, context=None,
31+ query='', query_params=()):
32+ """ compute the balance, debit and/or credit for the provided
33+ account ids
34
35+ Arguments:
36+ `ids`: account ids
37+ `field_names`: the fields to compute (a list of any of
38+ 'balance', 'debit' and 'credit')
39+ `arg`: unused fields.function stuff
40+ `query`: additional query filter (as a string)
41+ `query_params`: parameters for the provided query string
42+ (__compute will handle their escaping) as a
43+ tuple
44+ """
45+ mapping = {
46+ 'debit': "COALESCE(SUM(l.debit), 0) AS debit",
47+ 'credit': "COALESCE(SUM(l.credit), 0) AS credit",
48+ # by convention, foreign_balance is 0 when the account has no secondary currency, because the amounts may be in different currencies
49+ # Calculate is wrong with these new sql
50+ #'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",
51+ 'foreign_balance': "CASE WHEN MIN(parent_currency_id) IS NULL THEN 0 ELSE COALESCE(SUM(l.amount_currency), 0) END AS foreign_balance"
52+ }
53+ mapping2 = {
54+ 'balance': "debit-credit AS balance"
55+ }
56+
57+ accounts = {}
58+ aml_query = self.pool.get('account.move.line')._query_get(cr, uid, context=context)
59+
60+ wheres = [""]
61+ if query.strip():
62+ wheres.append(query.strip())
63+ if aml_query.strip():
64+ wheres.append(aml_query.strip())
65+ filters = " AND ".join(wheres)
66+ _logger.debug('Filters: %s',(filters))
67+ #children_and_consolidated = self._get_children_and_consol(
68+ #cr, uid, ids, context=context)
69+ children_and_consolidated = ids #Now we don't need get children_and_consolidated from function. Now is from query b-tree
70+ if children_and_consolidated:
71+ request= '''SELECT *, ''' +\
72+ ', '.join( mapping2.values() ) +\
73+ '''
74+ FROM (
75+ SELECT account_child_and_consolidated.parent_id AS id,''' +\
76+ ', '.join( mapping.values() ) +\
77+ '''
78+ FROM (
79+ SELECT account_child_vw.parent_id, COALESCE( account_consolidated_vw.child_id, account_child_vw.child_id) AS child_id
80+ , COALESCE( account_consolidated_vw.parent_currency_id, account_child_vw.parent_currency_id) AS parent_currency_id
81+ FROM (
82+ SELECT aa_tree_1.id AS parent_id, aa_tree_2.id AS child_id, aa_tree_1.currency_id AS parent_currency_id
83+ FROM account_account aa_tree_1
84+ LEFT OUTER JOIN account_account aa_tree_2
85+ ON aa_tree_2.parent_left
86+ BETWEEN aa_tree_1.parent_left AND aa_tree_1.parent_right
87+ ) account_child_vw
88+ LEFT OUTER JOIN
89+ (
90+ SELECT aa_tree_1.id AS parent_id, aa_tree_4.id AS child_id, aa_tree_1.currency_id AS parent_currency_id
91+ FROM account_account_consol_rel
92+ INNER JOIN account_account aa_tree_1
93+ ON aa_tree_1.id = account_account_consol_rel.child_id
94+ INNER JOIN account_account aa_tree_2
95+ ON aa_tree_2.id = account_account_consol_rel.parent_id
96+ LEFT OUTER JOIN account_account aa_tree_3
97+ ON aa_tree_3.parent_left
98+ BETWEEN aa_tree_1.parent_left AND aa_tree_1.parent_right
99+ LEFT OUTER JOIN account_account aa_tree_4
100+ ON aa_tree_4.parent_left
101+ BETWEEN aa_tree_2.parent_left AND aa_tree_2.parent_right
102+ ) account_consolidated_vw
103+ ON account_child_vw.child_id = account_consolidated_vw.parent_id
104+ ) account_child_and_consolidated
105+ LEFT OUTER JOIN account_move_line l
106+ ON l.account_id = account_child_and_consolidated.child_id
107+ ''' \
108+ 'WHERE account_child_and_consolidated.parent_id IN %s ' \
109+ + filters + \
110+ 'GROUP BY account_child_and_consolidated.parent_id ) subvw'
111+ params = (tuple(children_and_consolidated),) + query_params
112+ cr.execute(request, params)
113+ _logger.debug('Status: %s'%cr.statusmessage)
114+
115+ for res1 in cr.dictfetchall():
116+ accounts[res1['id']] = res1
117+ #TODO: Process multi-currency
118+ res = {}
119+ for id in ids:
120+ for field_name in field_names:
121+ res.setdefault(id, {})[field_name] = accounts.get(id, {}).get(field_name, 0.0)
122+ return res
123+
124+ ##Switch function for test
125+ #__compute = __compute_ORIGINAL
126+ __compute = __compute_BTREE
127+
128 def _get_company_currency(self, cr, uid, ids, field_name, arg, context=None):
129 result = {}
130 for rec in self.browse(cr, uid, ids, context=context):
131
132=== added file 'account/test/account_compute.yml'
133--- account/test/account_compute.yml 1970-01-01 00:00:00 +0000
134+++ account/test/account_compute.yml 2012-09-30 22:10:28 +0000
135@@ -0,0 +1,111 @@
136+-
137+ Verify account compute of supplier invoice
138+-
139+ !python {model: account.account, id: account_account_expense_consolidated0}:
140+ # Temporarily stop deferring parent_store computation
141+ self.pool._init = False
142+-
143+ I verify that amount of account expense is calculated correctly.
144+-
145+ !assert {model: account.account, id: account.a_expense, string: Amount of account expense is not calculated correctly.}:
146+ - int( debit ) == 3100 and int( credit ) == 0 and int( balance ) == 3100
147+-
148+ I verify that amount of account parent expense is calculated correctly.
149+-
150+ !assert {model: account.account, id: account.a_expense, string: Amount of account parent expense is not calculated correctly.}:
151+ - int( parent_id.debit ) == 3100 and int( parent_id.credit ) == 0 and int( parent_id.balance ) == 3100
152+-
153+ I create a consolidated expense account
154+-
155+ !record {model: account.account, id: account_account_expense_consolidated0}:
156+ code: 90001_test_consolidated_expense
157+ company_id: base.main_company
158+ currency_mode: current
159+ name: Consolidated Expense For Test
160+ type: consolidation
161+ user_type: account.data_account_type_view
162+ child_consol_ids:
163+ - account.a_expense
164+ parent_id: account.chart0
165+-
166+ I verify that debit amount of account consolidated expense is calculated correctly.
167+-
168+ !assert {model: account.account, id: account_account_expense_consolidated0, string: Amount of account consolidated expense is not calculated correctly.}:
169+ - int( debit ) == 3100 and int( credit ) == 0 and int( balance ) == 3100
170+
171+#Verify account compute of customer invoice
172+-
173+ I verify that amount of account income is calculated correctly.
174+-
175+ !assert {model: account.account, id: account.a_sale, string: Amount of account income is not calculated correctly.}:
176+ - int( debit ) == 0 and int( credit ) == 9000 and int( balance ) == -9000
177+-
178+ I verify that amount of account parent income is calculated correctly.
179+-
180+ !assert {model: account.account, id: account.a_sale, string: Amount of account parent income is not calculated correctly.}:
181+ - int( parent_id.debit ) == 0 and int( parent_id.credit ) == 9000 and int( parent_id.balance ) == -9000
182+-
183+ I create a consolidated income account
184+-
185+ !record {model: account.account, id: account_account_income_consolidated0}:
186+ code: 90002_test_consolidated_income
187+ company_id: base.main_company
188+ currency_mode: current
189+ name: Consolidated income For Test
190+ type: consolidation
191+ user_type: account.data_account_type_view
192+ child_consol_ids:
193+ - account.a_sale
194+ parent_id: account.chart0
195+-
196+ I verify that debit amount of account consolidated income is calculated correctly.
197+-
198+ !assert {model: account.account, id: account_account_income_consolidated0, string: Amount of account consolidated income is not calculated correctly.}:
199+ - int( debit ) == 0 and int( credit ) == 9000 and int( balance ) == -9000
200+-
201+ I create a account Debtor empty
202+-
203+ !record {model: account.account, id: account_account_debtor_2}:
204+ code: 90003_test_debtor2
205+ company_id: base.main_company
206+ currency_mode: current
207+ name: Debtor2
208+ type: receivable
209+ user_type: account.data_account_type_receivable
210+ parent_id: account.chart0
211+ currency_id: base.USD
212+-
213+ I assign manually new account to invoice currency
214+-
215+ !python {model: account.invoice}: |
216+ writed = self.write(cr, uid, ref('account_invoice_currency'), {
217+ 'account_id': ref('account_account_debtor_2'),
218+ })
219+ assert writed, "Account has not been assigned correctly"
220+
221+-
222+ I validate invoice currency by clicking on Create button
223+-
224+ !workflow {model: account.invoice, action: invoice_open, ref: account_invoice_currency}
225+-
226+ I check that the invoice state is "Open"
227+-
228+ !assert {model: account.invoice, id: account_invoice_currency}:
229+ - state == 'open'
230+-
231+ I check that now there is a move attached to the invoice
232+-
233+ !python {model: account.invoice}: |
234+ acc_id=self.browse(cr, uid, ref("account_invoice_currency"))
235+ assert acc_id.move_id, "Move not created for open invoice"
236+-
237+ I verify with multi-currency compute amounts
238+-
239+ !assert {model: account.account, id: account_account_debtor_2, string: Wrong multi-currency compute amounts}:
240+ - int( debit ) == 450 and int( credit ) == 0 and int( balance ) == 450
241+
242+-
243+ !python {model: account.account, id: account_account_expense_consolidated0}:
244+ # Restore deferring parent_store computation
245+ self.pool._init = True
246+

Subscribers

People subscribed via source and target branches

to all changes: