Skip to content

[17.0] [FIX] res_partner_operating_unit: performance due to missing indexes - #837

Open
ivantodorovich wants to merge 1 commit into
OCA:17.0from
camptocamp:17.0-fix-partner-operating-unit
Open

[17.0] [FIX] res_partner_operating_unit: performance due to missing indexes#837
ivantodorovich wants to merge 1 commit into
OCA:17.0from
camptocamp:17.0-fix-partner-operating-unit

Conversation

@ivantodorovich

@ivantodorovich ivantodorovich commented Apr 8, 2026

Copy link
Copy Markdown

There are missing indexes in a Many2many relation table because of this hook.

You see, the ORM takes care of creating indexes for relation tables, but only if its the one creating the table:

The hook were manually creating the relation table and so bypassing the index creation, leading to performance issues (taking up to 15seconds to open the Contacts menu in our production db)

Looking into why the hook was created in the first place, I think it was wrongly included in one of the migrations.

You see, this code was originally proposed in 12.0, here:

However, that PR was never merged. There was even a review explicitly stating the PR should not be merged:

Nonetheless, the commit was included in the migration to 14.0 -- which was never merged anyway:

Later, though, a migration to 17.0 proposal was made based on the mentioned migration to 14.0 work


As for the commit itself, I have to say I find no reason to make the operating_unit_ids field required, nor to hack the ir.rule. Maybe it was needed in 12.0 for some reason.. but not now.

The proposed solution is:

  • Remove the required=True in the res.partner.operating_unit_ids field. The existing ir.rule already considers unset values.
  • Remove the pre_init_hook that manually.
  • For existing deployments, the migration script takes care of creating the missing indexes

@vvrossem vvrossem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LG!

@SilvioC2C SilvioC2C left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@AaronHForgeFlow AaronHForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair enough

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions Bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Aug 9, 2026
@ivantodorovich

Copy link
Copy Markdown
Author

@AaronHForgeFlow is it ok to merge ? 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved stale PR/Issue without recent activity, it'll be soon closed automatically.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants