[FIX] orm: in recompute_fields, avoid memory error from postgresql

[cawo-odoo]

improves 13adede

Queries run through client-side cursors will make postgresql materialze the whole of the result immediately (which is actually, why cr.rowcount is always available right after execute in this case). With server side cursors (named cursors) on the other hand, tuples are materialized when they are fetched. This is why running the query for ids through the client-side cursor just to be able to access cr.rowcount, can cause an out-of-memory exception from PostgreSQL.

We fix this by wrapping the query in a CREATE TABLE AS statement that inserts returned ids into a temporary table. We then use a named_cursor to fetch ids from this table in chunks, server-side.

Another approach would have been to just wrap the query in a SELECT count(*) query and run this once to get the count. The approach using CREATE TABLE AS has been chosen over that solution to support queries that include DML statements (e.g. UPDATE ... RETURNING) that affect the results of the compute, as it allows us to run the query on the main (client) cursor, while still using a named_cursor for fetching the ids memory-efficiently.

[robodoo]

[cawo-odoo]

Testing this now, we'll know in ~6h

[cawo-odoo]

Tests with the currently effected DB look good (on PG17).

Last fixup is to fix PG12 - PG15 (require an alias for aggreagates on a subquery)

[cawo-odoo]

pushed fixup to take this case into regard: https://runbot.odoo.com/runbot/build/89536248

[KangOl]

upgradeci retry with always only account hr_recruitment_extract hr_expense sale_stock sale_subscription crm l10n_it_edi sale purchase mrp

[aj-fuentes]

Make it fail if the query is wrong. Avoid id-injection to the ORM.

Suggested change

	ncr.execute("SELECT * FROM _upgrade_rf")
	ncr.execute("SELECT id::int FROM _upgrade_rf")

[KangOl]

I won't cast it to int. On some (really) huge tables, it's not excluded that they has to promote the id to bigint.

[aj-fuentes]

This was an optional stuff. But note that we could get select name from table as query. Whether this is an issue for the ORM or not it would depend on the running version... The error later could be harder to understand than a type cast error.

[cawo-odoo]

This was an optional stuff. But note that we could get select name from table as query. Whether this is an issue for the ORM or not it would depend on the running version... The error later could be harder to understand than a type cast error.

If we really want to do that, I would make it such that it runs a normal create table first. In there, we can specify the column type and create the PK all in one go.

[aj-fuentes]

We don't need to :) I just wanted a quick/simple way to avoid the issue. We could cast to bigint as well, but it may have some perf impact.

[cawo-odoo]

Last fixup is to fix:

  File "/upgrade-util/src/util/orm.py", line 292, in recompute_fields
    cr.execute(format_query(cr, "CREATE UNLOGGED TABLE _upgrade_rf(id) AS ({})", query))
  File "/tmp/tmp2rppghvk/odoo/17.0/odoo/sql_db.py", line 335, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.SyntaxError: syntax error at or near "DELETE"
LINE 2:             DELETE FROM account_edi_document aed
                    ^

[cawo-odoo]

aj-fuentes approved these changes

🎉 Test on big DB is still running, results should come in before tomorrow.

EDIT: bundle tested successfully on big DB. @KangOl

[cawo-odoo]

Pushed a small simplification that came to mind while applying the same idea to iter_browse

[aj-fuentes]

I think your last patch is leaving behind the table _upgrade_rf when count is zero.

[cawo-odoo]

I think your last patch is leaving behind the table _upgrade_rf when count is zero.

Oh, well. I forgot (to think - thanks for doing it for me) about the early return ... too bad then. Changing it back.

[cawo-odoo]

Updated commit message. From my side this is ready and it is still the same what Alvaro approved already.

[KangOl]

@robodoo r+