[ADD] util/update_table_from_dict

[Pirols]

A recurrent challenge in writing upgrade scripts is that of updating values in a table based on some form of already available mapping from the id (or another identifier) to the new value, this is often addressed with an iterative solution in the form:

for key, value in mapping.items():
    cr.execute(
        """
        UPDATE table
           SET col = %s
         WHERE key_col = %s
        """,
        [value, key],
    )

or in a more efficient (only issuing a single query) but hacky way:

cr.execute(
    """
    UPDATE table
       SET col = (%s::jsonb)->>(key_col::text)
     WHERE key_col = ANY(%s)
    """,
    [json.dumps(mapping), list(mapping)],
)

With the former being ineffective for big mappings and the latter often requiring some comments at review time to get it right. This commit introduces a util meant to make it easier to efficiently perform such updates.

[robodoo]

[Pirols]

Usage example: https://github.com/odoo/upgrade/pull/8079

[KangOl]

upgradeci retry with always only base

[KangOl]

I would also like to have a validation of the mapping

Suggested change

	_validate_table(table)
	_validate_table(table)
	assert isinstance(mapping, dict) and all(isinstance(key, int) and isinstance(value, (list, tuple)) for key, value in mapping.items())

[Pirols]

Do we want to keep key_col and limit its potential targets to integer columns or remove it entirely?

[aj-fuentes]

I think key_col should always be an int, but I like to have it variable. It would allow to update a table based in a FK instead of a PK. To decide you can grep the repo and see if we currently use the dict trick for other columns than id... I think we did but my memory could betray me.

[Pirols]

From a relatively quick search I found 4 usages of the json trick on columns other than id:

• varchar column (2 usages): https://github.com/odoo/upgrade/blob/dad3dd7a1554c526d37fd5f61e6f4a9e18667e56/migrations/sale_subscription/saas~15.3.1.1/pre-migrate.py#L588-L610 and https://github.com/odoo/upgrade/blob/dad3dd7a1554c526d37fd5f61e6f4a9e18667e56/migrations/sale_subscription/saas~15.3.1.1/pre-migrate.py#L612-L634
• varchar column: https://github.com/odoo/upgrade/blob/dad3dd7a1554c526d37fd5f61e6f4a9e18667e56/migrations/account/saas~17.4.1.2/pre-migrate.py#L51-L63
• integer column: https://github.com/odoo/upgrade/blob/dad3dd7a1554c526d37fd5f61e6f4a9e18667e56/migrations/l10n_in_hr_payroll/saas~17.5.1.0/post-migrate.py#L21-L43

[aj-fuentes]

OK, then maybe it's worth taking the type from the key_col in the table directly.

[Pirols]

That should not be necessary since JSONB_EACH returns the key as a text¹.
Unless we want to validate the type of the keys of mapping. In which case I would need to define (or reuse, if already available) a mapping from pg types to python types. Wdyt?

Footnotes

1. https://www.postgresql.org/docs/current/functions-json.html#FUNCTIONS-JSON-PROCESSING-TABLE ↩

[aj-fuentes]

Well I just noticed you are casting the key_col to text before comparing with the actual json dict key, then it's fine. :) What I had in mind was casting the dict key to the column type. In principle my suggestion was to block/fail if we get an invalid json dict (incompatible with the table), I have no strong opinion against current approach TBH. @KangOl wdyt?