Skip to content
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

Forward Port of #35411 to saas-12.3 #36769

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Sep 12, 2019

Check group integrity constraints

I should have added that info in the first commit, but the main issue that was pointed by Odony in an informal communication was that check_one_user_type does not work well, in the sense that this constraint can be bypassed within Odoo.
The reason is that when modifying groups via the settings, the users are given groups by the implied group mechanism.
Moreover a direct SQL query bypass an explicit write on the users field of groups, which is constrained.
This PR is to restore the lost integrity check with a minimal impact on performance.

On heavy load, the performance impact is around 6% (where half are the read on the relevant group ids and such and half is on the _fast_check..., e.g. the query checking the constraint itself.

Forward-Port-Of: #35411

@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Sep 12, 2019
Since commit 5f12e24, the write of implied groups is done directly in SQL,
bypassing the ORM writes, and thus bypassing the python constraints checks.
In particular such a constraint was added in f206714.
We add back an explicit check of this constraint;
however we rewrite it in SQL to be fast.

closes odoo#35411

Signed-off-by: Nans Lefebvre (len) <len@odoo.com>
@len-foss len-foss force-pushed the saas-12.3-12.0-opw2041606-grpx-len-FJCB2URD-forwardport branch from 2bacc04 to aa05b4e Compare September 12, 2019 14:05
@odoo odoo deleted a comment from robodoo Sep 12, 2019
@len-foss
Copy link
Contributor

robodoo r+

@robodoo robodoo added r+ 👌 CI 🤖 Robodoo has seen passing statuses labels Sep 12, 2019
robodoo pushed a commit that referenced this pull request Sep 12, 2019
Since commit 5f12e24, the write of implied groups is done directly in SQL,
bypassing the ORM writes, and thus bypassing the python constraints checks.
In particular such a constraint was added in f206714.
We add back an explicit check of this constraint;
however we rewrite it in SQL to be fast.

closes #35411

closes #36769

Signed-off-by: Nans Lefebvre (len) <len@odoo.com>
Signed-off-by: Nans Lefebvre (len) <len@odoo.com>
@robodoo
Copy link
Contributor

robodoo commented Sep 12, 2019

Merged at 459e6dc, thanks!

@robodoo robodoo closed this Sep 12, 2019
@len-foss len-foss deleted the saas-12.3-12.0-opw2041606-grpx-len-FJCB2URD-forwardport branch September 13, 2019 06:03
@len-foss len-foss changed the title Forward Port of #35411 to saas-12.3 (failed) Forward Port of #35411 to saas-12.3 Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants