Skip to content

[FIX] orm: avoid groupby on models with custom _select#246333

Closed
kmagusiak wants to merge 1 commit intoodoo:saas-19.1from
odoo-dev:saas-19.1-avoid-groupby-krma
Closed

[FIX] orm: avoid groupby on models with custom _select#246333
kmagusiak wants to merge 1 commit intoodoo:saas-19.1from
odoo-dev:saas-19.1-avoid-groupby-krma

Conversation

@kmagusiak
Copy link
Copy Markdown
Contributor

Add a context to know if we are grouppnig. This allows us to stop _search methods which need to search and filter manually all records.

For example, when asking if a field is groupable,
_description_groupable will run a dummy search for many2many fields, for a normal user searching on ir.attachment, it would search for all attachments and try to filter them in memory leading to OOM error.


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo
Copy link
Copy Markdown
Contributor

robodoo commented Jan 29, 2026

Pull request status dashboard

Comment thread odoo/orm/models.py Outdated
Comment thread odoo/orm/models.py Outdated
Comment thread odoo/addons/base/models/ir_attachment.py
@kmagusiak kmagusiak force-pushed the saas-19.1-avoid-groupby-krma branch from ffc2717 to 455283c Compare January 29, 2026 16:07
Comment on lines +616 to +617
if self.env.context.get('_read_groupby'):
raise ValueError("Cannot group by ir.attachment")
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.

Is it not only the search_fetch combined with _as_query that causes an issue?

Can't we just move this condition later, if limit is None?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this function is also called in read group and in other places of the orm. Technically, the condition can be moved, but it results in some domains working and other not depending on the optimization (quick could change between versions). It's easier to say don't group on all attachments (what would be the use case?)

(still a draft)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I won't move the condition later to keep things consistent.

@kmagusiak kmagusiak force-pushed the saas-19.1-avoid-groupby-krma branch 2 times, most recently from be8f461 to 6bc9e04 Compare February 3, 2026 08:49
@kmagusiak kmagusiak requested a review from rco-odoo February 5, 2026 12:25
Add a context to know if we are grouppnig. This allows us to stop
`_search` methods which need to search and filter manually all records.

For example, when asking if a field is groupable,
`_description_groupable` will run a dummy search for many2many fields,
for a normal user searching on ir.attachment, it would search for all
attachments and try to filter them in memory leading to OOM error.
@kmagusiak kmagusiak force-pushed the saas-19.1-avoid-groupby-krma branch from 6bc9e04 to 0fc4843 Compare February 5, 2026 12:25
Copy link
Copy Markdown
Member

@rco-odoo rco-odoo left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@kmagusiak kmagusiak marked this pull request as ready for review February 5, 2026 13:22
@rco-odoo
Copy link
Copy Markdown
Member

rco-odoo commented Feb 5, 2026

@robodoo delegate+

@C3POdoo C3POdoo requested review from a team, HydrionBurst and rugo-odoo and removed request for a team February 5, 2026 13:41
@kmagusiak
Copy link
Copy Markdown
Contributor Author

@robodoo r+

robodoo pushed a commit that referenced this pull request Feb 5, 2026
Add a context to know if we are grouppnig. This allows us to stop
`_search` methods which need to search and filter manually all records.

For example, when asking if a field is groupable,
`_description_groupable` will run a dummy search for many2many fields,
for a normal user searching on ir.attachment, it would search for all
attachments and try to filter them in memory leading to OOM error.

closes #246333

Signed-off-by: Krzysztof Magusiak (krma) <krma@odoo.com>
@robodoo robodoo closed this Feb 5, 2026
@kmagusiak kmagusiak deleted the saas-19.1-avoid-groupby-krma branch February 6, 2026 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants