Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] account: avoid adding an extra group_by to break method signatu… #51497

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

fpodoo
Copy link
Contributor

@fpodoo fpodoo commented May 18, 2020

[FIX] account: avoid adding an extra group_by to break method signature (and offset, limits)
[IMP] account: account.invoice.report SPEED IMPROVEMENT: removed 4 unused JOIN clauses
[IMP] account: account.invoice.report removed unnecessary fields:

currency_id: makes no sense as the values are not in this currency; confusing
commercial_partner_id: users don't even know the difference with partner_id
quantities: normalize to "Reference UoM" for the category instead of "Product UoM"
LRU Cache is risky, if more than 32 currencies (probably not, but still)

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

…re (and offset, limits)

[IMP] account: account.invoice.report SPEED IMPROVEMENT: removed 4 unused JOIN clauses
[IMP] account: account.invoice.report removed unnecessary fields

currency_id: makes no sense as the values are not in this currency; confusing
commercial_partner_id: users don't even know the difference with partner_id
quantities: reference to "Reference UoM" instead of "Product UoM"
LRU Cache is risky, if more than 32 currencies (probably not, but still)
@C3POdoo C3POdoo added the RD research & development, internal work label May 18, 2020
@william-andre
Copy link
Contributor

william-andre commented May 19, 2020

It can't work because the SQL view doesn't depend on the context (active_company_ids), and you are trying to do the conversions inside the view.
#51122 Is actually the only approach I can think of that makes sense.

@nim-odoo
Copy link
Contributor

@william-andre

Actually we may give a shot to the following:

SELECT
    line.id,
    line.move_id,
    line.product_id,
    line.account_id,
    line.analytic_account_id,
    line.journal_id,
    line.company_id,
    currency.company_id company_to_id,
    line.company_currency_id,
    move.state,
    move.move_type,
    move.partner_id,
    move.invoice_user_id,
    move.fiscal_position_id,
    move.payment_state,
    move.invoice_date,
    move.invoice_date_due,
    partner.country_id AS country_id,
    template.categ_id  AS product_categ_id,
    line.quantity / NULLIF(COALESCE(uom_line.factor, 1), 0.0) AS quantity,
    -line.balance / COALESCE(currency.rate,1) AS price_subtotal,
    -line.balance / NULLIF(COALESCE(uom_line.factor, 1), 0.0) / COALESCE(currency.rate,1) AS price_average
FROM account_move_line line
    LEFT JOIN res_company ON line.company_id = res_company.id
    LEFT JOIN res_currency ON line.company_currency_id = res_currency.id
    LEFT JOIN res_partner partner ON partner.id = line.partner_id
    LEFT JOIN product_product product ON product.id = line.product_id
    LEFT JOIN product_template template ON template.id = product.product_tmpl_id
    LEFT JOIN uom_uom uom_line ON uom_line.id = line.product_uom_id
    INNER JOIN account_move move ON move.id = line.move_id
    LEFT JOIN (
	    SELECT res_company.id as company_id, res_currency.id AS currency_id, COALESCE((
	        SELECT DISTINCT ON (company_id, currency_id) rate
	        FROM res_currency_rate AS cr_rate
	        WHERE cr_rate.company_id = res_company.id
	            AND cr_rate.currency_id = res_currency.id
	            AND cr_rate.name < now()
	        ORDER BY cr_rate.currency_id, cr_rate.company_id, cr_rate.name DESC), 1.0
	    ) AS rate
	    FROM res_company, res_currency
        WHERE res_currency.id in (SELECT currency_id FROM res_company)
    ) currency ON (line.company_currency_id = currency.currency_id)
WHERE move.move_type IN ('out_invoice', 'out_refund', 'in_invoice', 'in_refund', 'out_receipt', 'in_receipt')
    AND line.account_id IS NOT NULL
    AND NOT line.exclude_from_invoice_tab;

(the LEFT JOIN on the currency table may need to be reworked, but as is it seems to work)

This way, you duplicate all lines of the report that you filter in the read_group:

def read_group(self, domain, fields, groupby, offset=0, limit=None, orderby=False, lazy=True):
    result = super(AccountInvoiceReport, self).read_group(domain+[('company_to_id','=',self.env.company.id)], fields, groupby, offset, limit, orderby, False)
    return result

@william-andre
Copy link
Contributor

Ah, I went too quickly. It is a good idea indeed.
I don't think it would be a problem, but to be on the safe side, we should test that the LEFT JOIN (SELECT ... (SELECT DISTINCT ...) WHERE ... IN (SELECT ...) is not bad for the query planner.

@william-andre
Copy link
Contributor

We can also round the values in the SQL query so that we don't need the rounding part in the read_group

@nim-odoo
Copy link
Contributor

note to myself:

SELECT DISTINCT ON (cc_matrix.company_id, cc_matrix.currency_id) 
cc_matrix.company_id, 
cc_matrix.currency_id, 
COALESCE(rate, 1.0) 
FROM   (SELECT res_company.id  AS company_id, 
               res_currency.id AS currency_id 
        FROM   res_company, 
               res_currency 
        WHERE  res_currency.id IN (SELECT currency_id 
                                   FROM   res_company)) cc_matrix 
       LEFT JOIN res_currency_rate cr_rate 
              ON cc_matrix.company_id = cr_rate.company_id 
                 AND cc_matrix.currency_id = cr_rate.currency_id 
ORDER  BY cc_matrix.company_id, 
          cc_matrix.currency_id, 
          name; 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants