Merge lp:~openerp-dev/openobject-addons/trunk-bug-922621-mdi into lp:openobject-addons

[DJ Patel (OpenERP)]

Hello Sir,

I have fix the issue: https://bugs.launchpad.net/openobject-addons/+bug/922621 "foreign_balance should not be computed for account with no secondary currency".

Thanks and Regards,

Divyesh Makwana(MDI)

[Raphael Collet (OpenERP) (rco-openerp)]

It works. I added some comment to explain the change.

Thanks,
Raphael

review: Approve

[Olivier Dony (Odoo) (odo-openerp)]

For the record, here is a comparison of explain plans simulating the sub-select used in the patch and a full JOIN against account_account. The actual times are those of a sample database with 1071 account_move_lines in account 838.
It seems the sub-select is a bit cheaper than the full-blown join, both in the estimated and actual costs:

# -- SUB-SELECT VERSION
# explain analyze select (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 from account_move_line l where l.account_id = 838 group by l.account_id;
                                                                      QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------
 GroupAggregate (cost=9.45..525.70 rows=2 width=20) (actual time=3.715..3.715 rows=1 loops=1)
   -> Bitmap Heap Scan on account_move_line l (cost=9.45..507.98 rows=154 width=20) (actual time=0.357..2.824 rows=1071 loops=1)
         Recheck Cond: (account_id = 838)
         -> Bitmap Index Scan on account_move_line_account_id_index (cost=0.00..9.41 rows=154 width=0) (actual time=0.248..0.248 rows=1205 loops=1)
               Index Cond: (account_id = 838)
   SubPlan 1
     -> Index Scan using account_account_pkey on account_account (cost=0.00..8.27 rows=1 width=4) (actual time=0.023..0.024 rows=1 loops=1)
           Index Cond: (id = $2)
 Total runtime: 3.781 ms
(9 rows)

# -- JOIN VERSION
# explain analyze select CASE WHEN a.currency_id is null then 0 else COALESCE(SUM(l.amount_currency), 0) end as foreign_balance from account_move_line l join account_account a on (l.account_id = a.id) where l.account_id = 838 group by a.id, a.currency_id;
                                                                         QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------
 HashAggregate (cost=519.33..519.34 rows=1 width=24) (actual time=3.958..3.958 rows=1 loops=1)
   -> Nested Loop (cost=9.45..517.79 rows=154 width=24) (actual time=0.373..3.201 rows=1071 loops=1)
         -> Index Scan using account_account_pkey on account_account a (cost=0.00..8.27 rows=1 width=8) (actual time=0.026..0.028 rows=1 loops=1)
               Index Cond: (id = 838)
         -> Bitmap Heap Scan on account_move_line l (cost=9.45..507.98 rows=154 width=20) (actual time=0.341..2.716 rows=1071 loops=1)
               Recheck Cond: (l.account_id = 838)
               -> Bitmap Index Scan on account_move_line_account_id_index (cost=0.00..9.41 rows=154 width=0) (actual time=0.233..0.233 rows=1205 loops=1)
                     Index Cond: (l.account_id = 838)
 Total runtime: 4.029 ms
(9 rows)

PS: the "id IN (l.account_id))" in the merge proposal is a bit weird and should have been written as "id = l.account_id"

=== modified file 'account/account.py'
--- account/account.py	2012-01-31 13:36:57 +0000
+++ account/account.py	2012-02-02 06:22:21 +0000
@@ -278,7 +278,7 @@
                        "- COALESCE(SUM(l.credit), 0) as balance",
             'debit': "COALESCE(SUM(l.debit), 0) as debit",
             'credit': "COALESCE(SUM(l.credit), 0) as credit",
-            'foreign_balance': "COALESCE(SUM(l.amount_currency), 0) as foreign_balance",
+            '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 ",
         }
         #get all the necessary accounts
         children_and_consolidated = self._get_children_and_consol(cr, uid, ids, context=context)