Skip to content

[FIX] util/records: deduplicate before batch update #243

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

Closed
wants to merge 3 commits into from

Conversation

aj-fuentes
Copy link
Contributor

When updating record references we need to dedup uniq indexes. In the case of inderect references the case when two old references become the same new reference was missing.

Example to trigger the issue

test_15=> select * from mail_followers where res_model='res.partner.bank'
+----+------------------+--------+------------+
| id | res_model        | res_id | partner_id |
|----+------------------+--------+------------|
| 5  | res.partner.bank | 2      | 3          |
| 7  | res.partner.bank | 3      | 3          |
+----+------------------+--------+------------+
test_15=> select id,sanitized_acc_number,partner_id,company_id from res_partner_bank
+----+----------------------+------------+------------+
| id | sanitized_acc_number | partner_id | company_id |
|----+----------------------+------------+------------|
| 1  | 1                    | 3          | 1          |
| 2  | 1                    | 3          | 2          |
| 3  | 1                    | 3          | 3          |
+----+----------------------+------------+------------+

In such case the record replacer will replace references to the partner bank as in the mapping {2:1, 3:1}. But this causes an error

2025-03-07 17:12:12,145 502959 ERROR test_15_16 odoo.sql_db: bad query:

            UPDATE "mail_followers" t
               SET "res_model" = 'res.partner.bank'
                   , "res_id" = _upgrade_rrr.new
              FROM _upgrade_rrr
             WHERE "res_model"='res.partner.bank'
               AND _upgrade_rrr.old = res_id

                    AND
                        NOT EXISTS(SELECT 1 FROM mail_followers WHERE "res_model" = 'res.partner.bank' AND "res_id" = _upgrade_rrr.new AND "partner_id"=t."partner_id")
                    ;
                DELETE FROM mail_followers USING _upgrade_rrr WHERE "res_model"='res.partner.bank' AND res_id = _upgrade_rrr.old;

ERROR: duplicate key value violates unique constraint "mail_followers_mail_followers_res_partner_res_model_id_uniq"
DETAIL:  Key (res_model, res_id, partner_id)=(res.partner.bank, 1, 3) already exists.

@robodoo
Copy link
Contributor

robodoo commented Apr 4, 2025

Pull request status dashboard

@KangOl
Copy link
Contributor

KangOl commented Apr 4, 2025

upgradeci retry with always only base hr_recruitment sale_subscription helpdesk

@aj-fuentes aj-fuentes force-pushed the master-fix_rrr2-afu branch from 9ba343d to 0c74f32 Compare April 7, 2025 08:39
We perform some DELETEs in `replace_record_references_batch` as a
cleanup step. Thus it is better to ensure the mapping always change the
id.

Example:
https://github.com/odoo/upgrade-util/blob/dced6e030db111fab9446b323434e4c2e16bdd62/src/util/records.py#L1723
Modifying the underlying list of indirect references is useful for
making flows based on indirect references easier to test. Is is also
useful for the upgrade of custom modules after a major upgrade.
@aj-fuentes aj-fuentes force-pushed the master-fix_rrr2-afu branch 5 times, most recently from 20afd15 to 87c87b9 Compare April 10, 2025 11:04
@aj-fuentes aj-fuentes force-pushed the master-fix_rrr2-afu branch from 87c87b9 to 9d8928f Compare April 10, 2025 12:22
@aj-fuentes aj-fuentes force-pushed the master-fix_rrr2-afu branch 2 times, most recently from cfadb29 to 50d688d Compare April 10, 2025 13:50
@aj-fuentes aj-fuentes force-pushed the master-fix_rrr2-afu branch from 50d688d to d1775ac Compare April 10, 2025 14:17
`replace_record_references_batch` tries to ensure there are no
duplicates when replacing records. It fails when the duplicate is being
introduced as a result of two ids mapping to the same target id _and_ no
original conflicting record exists.
@aj-fuentes aj-fuentes force-pushed the master-fix_rrr2-afu branch from d1775ac to e2dea0e Compare April 10, 2025 19:18
Copy link
Contributor

@KangOl KangOl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robodoo rebase-ff r+

@robodoo
Copy link
Contributor

robodoo commented Apr 11, 2025

Merge method set to rebase and fast-forward.

robodoo pushed a commit that referenced this pull request Apr 11, 2025
We perform some DELETEs in `replace_record_references_batch` as a
cleanup step. Thus it is better to ensure the mapping always change the
id.

Example:
https://github.com/odoo/upgrade-util/blob/dced6e030db111fab9446b323434e4c2e16bdd62/src/util/records.py#L1723

Part-of: #243
Signed-off-by: Christophe Simonis (chs) <[email protected]>
robodoo pushed a commit that referenced this pull request Apr 11, 2025
Modifying the underlying list of indirect references is useful for
making flows based on indirect references easier to test. Is is also
useful for the upgrade of custom modules after a major upgrade.

Part-of: #243
Signed-off-by: Christophe Simonis (chs) <[email protected]>
@robodoo robodoo closed this in 0cd78cb Apr 11, 2025
@robodoo robodoo added the 18.3 label Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants