[trunk] foreign_balance should not be computed for account with no secondary currency

[Nhomar - Vauxoo]

Hello.

We are working on this Optimization:

https://code.launchpad.net/~vauxoo/openobject-addons/trunk-bug_887376-account_compute_with_btree-VAUXOO/+merge/89741

And we face a BIG problem with multicurrency feature just improved some little time ago.

In this line of account.py

'foreign_balance': "COALESCE(SUM(l.amount_currency), 0) as foreign_balance",

We see that this algorithm sum all multicurrency values and it is TOTALLY wrong, because i can have n the same account different vaules, and we NEVER can sum Apples with Pears.

How functionally you can try:

Make an move invoice on €.
Then Make an invoice to same partner in USD$
Make an invoice an you currency, (We Use VEF).

Then go to edit tree view for accounts and show the field "foreign_balance" with the view editor,

You can face that if pfor example you made 3 invoice for 100 CURRENCY, the total for the account REceivable or Payable related on this invoices.

The amount received is 300 (sum directly for 100€ 100$ and 100VEF)

-In V6 this feature is simply missing.-

I think it is a huge cancept mistake.

IMHO, we need to have from OpenERP-dev team a Blueprint related with the concept that they want to implement in this field, because in other way this field is really useless and affect the performance in calculation.

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

Hi,

The 'foreign_balance' function field represents the balance in the secondary currency for GL accounts which *do have* a secondary currency set (currency_id != False), and thus have all their journal items expressed in that same secondary currency.
It makes no sense for GL accounts which have no secondary currency, and thus may mix different currencies.

We should improve the __compute function to make sure the foreign_balance is always computed as 0 for GL accounts that don't have a secondary currency.
This may confuse other people tinkering with the low-level accounting database structure, so let's not consider this a Wishlist.

Thanks for reporting.

[Nhomar - Vauxoo (nhomar)]

OK.

Excellent, i got it.

Our Poitn is, if you set a simple sum, no matter where you use it , it will be bad calculated.....

Please, how we are working on this too!,

can you improve just:

'foreign_balance': "COALESCE(SUM(l.amount_currency), 0) as foreign_balance",

Whit the correct SQL part, to put inside our Algorithm please.

BTW.

One question is important:

If I have 10 aml with 5 different currencies and some in 0 (empty), how this Amount must be shown an P&L?

Is a important question that we need to document before program IMHO (as anglosaxon does some time).

This kind of features can brake all demo, if number doesn't comply with accounting specifications.

Regards, and thanks for your time men!!!

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

You could do it with a simple CASE in the SQL query, but you need the value of the "account_account.currency_id" column, which may not be currently included in the query - so a bit of refactoring may be needed.
Something like:
    'foreign_balance': "CASE WHEN account_account.currency_id is NULL THEN 0 ELSE COALESCE(SUM(l.amount_currency), 0) END as foreign_balance"

It's not currently used in P&L, but only in the special "Unrealized Gain or Loss" report in Multi-Currency reports.
It could be added in P&L for information purpose, but only for accounts with secondary currencies.

[DJ Patel (OpenERP) (mdi-openerp)]

Hello Nhomar Hernandez,

Thanks for reporting. The solution for this bug is proposed in the branch : https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-922621-mdi/

with following Revision ID and Number.

Revision ID : <email address hidden>
Revision Number : 6468

Thanks and Regards,

Divyesh Makwana(MDI)

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

merged in trunk
revno: 6469 [merge]
revision-id: <email address hidden>

[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, so they do not differ much, but still significantly.
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)