[FIX] m2o_to_x2m: fix dup key insertion

[rvalyi]

Hello, while trying to migrate a production database to v15, during the project_hr post-migration script (the one removed here OCA/project@5e35249):

def migrate(env, version):
    # Convert m2o to m2m in 'project.task'
    openupgrade.m2o_to_x2m(
        env.cr,
        env["project.task"],
        "project_task",
        "employee_ids",
        "employee_id",
    )

I got this error:

2024-09-01 18:36:23,184 153 INFO tec14-updated odoo.modules.migration: module project_hr: Running migration [15.0.1.0.0>] post-migration
2024-09-01 18:36:23,192 153 INFO tec14-updated OpenUpgrade: project_hr: post-migration script called with version 14.0.1.0.0
2024-09-01 18:36:23,193 153 ERROR tec14-updated odoo.sql_db: bad query:
            INSERT INTO hr_employee_project_task_rel (project_task_id, hr_employee_id)
            SELECT id, employee_id
            FROM project_task
            WHERE employee_id is not null

ERROR: duplicate key value violates unique constraint "hr_employee_project_task_rel_pkey"
DETAIL:  Key (project_task_id, hr_employee_id)=(197, 64) already exists.

2024-09-01 18:36:23,194 153 ERROR tec14-updated OpenUpgrade: Error after 0:00:00.000908 running
            INSERT INTO hr_employee_project_task_rel (project_task_id, hr_employee_id)
            SELECT id, employee_id
            FROM project_task
            WHERE employee_id is not null

2024-09-01 18:36:23,194 153 ERROR tec14-updated OpenUpgrade: project_hr: error in migration script /odoo/external-src/project/project_hr/migrations/15.0.1.0.0/post-migration.py: duplicate key value violates
unique constraint "hr_employee_project_task_rel_pkey"
DETAIL:  Key (project_task_id, hr_employee_id)=(197, 64) already exists.
2024-09-01 18:36:23,194 153 ERROR tec14-updated OpenUpgrade: project_hr: error in migration script /odoo/external-src/project/project_hr/migrations/15.0.1.0.0/post-migration.py: duplicate key value violates
unique constraint "hr_employee_project_task_rel_pkey"
DETAIL:  Key (project_task_id, hr_employee_id)=(197, 64) already exists.

2024-09-01 18:36:23,194 153 ERROR tec14-updated OpenUpgrade: duplicate key value violates unique constraint "hr_employee_project_task_rel_pkey"
DETAIL:  Key (project_task_id, hr_employee_id)=(197, 64) already exists.

Traceback (most recent call last):
  File "/env/lib/python3.8/site-packages/openupgradelib/openupgrade.py", line 2292, in wrapped_function
    func(
  File "/odoo/external-src/project/project_hr/migrations/15.0.1.0.0/post-migration.py", line 10, in migrate
    openupgrade.m2o_to_x2m(
  File "/env/lib/python3.8/site-packages/openupgradelib/openupgrade.py", line 1900, in m2o_to_x2m
    logged_query(
  File "/env/lib/python3.8/site-packages/openupgradelib/openupgrade.py", line 1703, in logged_query
    cr.execute(query, args)
  File "/env/lib/python3.8/site-packages/decorator.py", line 232, in fun
    return caller(func, *(extras + args), **kw)
  File "/odoo/src/odoo/sql_db.py", line 90, in check
    return f(self, *args, **kwargs)
  File "/odoo/src/odoo/sql_db.py", line 311, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "hr_employee_project_task_rel_pkey"
DETAIL:  Key (project_task_id, hr_employee_id)=(197, 64) already exists.

I asked chatGPT how I could avoid inserting duplicates and it suggested me this fix with EXCEPT. With this fix I got my migration working and the relation table was populated:

tec14-updated=> select count(*) from hr_employee_project_task_rel;
 count
-------
  1856
(1 row)

cc @aleuffre @pilarvargas-tecnativa @pedrobaeza

[rvalyi]

Today I wanted to reference the project_hr script and just see that @aleuffre removed it telling it was buggy OCA/project@5e35249

This fix in openupgradelib might still be interesting for other such situations. I let you guys be the judges.

[legalsylvain]

I asked chatGPT how I could avoid inserting duplicates and it suggested me this fix with EXCEPT.

hi @rvalyi. I'm not sure to understand. Do you understand the content of your PR ?

Thanks !

[rvalyi]

@legalsylvain I didn't know EXCEPT myself, but it seems it does the job. In the precise project_hr v15 migration script, understand that because of the computed field (see my previous link), the script was not required and "buggy". But still the m2o_to_x2m openupgradelib method has currently no guard against inserting duplicates and this may still be an interesting feature to have in the method. Thus it would only insert missing relations instead of failing hard if some relation was already inserted in the table previously; no matter how.

At this point I'm afraid chatGPT will explain the query better than me (the end is near for us :-) )

INSERT INTO hr_employee_project_task_rel (project_task_id, hr_employee_id)
SELECT id, employee_id
FROM project_task
WHERE employee_id IS NOT NULL
EXCEPT
SELECT project_task_id, hr_employee_id
FROM hr_employee_project_task_rel;

SELECT id, employee_id FROM project_task WHERE employee_id IS NOT NULL:

This part of the query selects all the records from the project_task table where the employee_id is not null. The selected columns are id (which represents project_task_id) and employee_id.
EXCEPT:

The EXCEPT clause compares the results of the two SELECT statements and returns only those rows that are present in the first SELECT but not in the second.
In this case, it filters out rows that already exist in the hr_employee_project_task_rel table based on the combination of project_task_id and hr_employee_id.
SELECT project_task_id, hr_employee_id FROM hr_employee_project_task_rel:

This part selects all the existing records from the hr_employee_project_task_rel table.
How EXCEPT Works
The EXCEPT clause essentially performs a set difference operation. It subtracts the set of rows in the second SELECT statement (existing entries in hr_employee_project_task_rel) from the set of rows in the first SELECT statement (potential new entries from project_task).

If a row from the first SELECT exists in the second SELECT, it will be excluded from the result, meaning it won't be inserted again, preventing duplicates.

[antoniospneto]

LGTM

[pedrobaeza]

The only way I see for having duplicates is because you are inserting records in an existing m2m table, which is a case you must handle other way. If not, a many2one (one value) is not able to produce duplicates when inserting it in the m2m table. Can you explain the use case where it produces duplicates? The project_hr case that you linked was happening because the compute method already inserted the m2m lines, so that's something legit and the script indeed should be removed (or prevent previously the compute method triggering), but it's not correct to do this prevention to silently bypass programming errors.

Closing for now, unless a valid use case arises.