Community: Framework mailing list archives

expert-framework@mail.odoo.com

Re: Enhancment proposal, new domain operator "parent_of"

by
Mohammad Alhashash
- 05/13/2015 05:55:43
Hi Oliver,

On 13/05/15 03:12, Olivier Dony wrote:
<blockquote cite="mid:CAP4GjTJ=v=P6YfhtytbvrHr+doa=E7+4vRN0VJ0tM=4DKz7YhQ@mail.gmail.com" type="cite">
Hi Mohammad,

On Sat, May 9, 2015 at 12:12 PM, Mohammad Alhashash
<alhashash@alhashash.net> wrote:
> I'd like to purpose a new operator "parent_of" with similar semantics to
> "child_of" operator but it would traverse parents instead of children.
>
> Here is an implementation based on master:
> https://github.com/alhashash/odoo/tree/master-new_operator_parent_of
>
> If the concept is accepted, I'll add unit tests and documentation updates to
> make a pull request.

The concept is quite fine indeed, and to say the truth, I have had the
'parent_of' operator in my personal wish/todo list for several years
now :-)
So I would definitely welcome a proper PR with tests and documentation
against the master branch.
I've a problem with the test. I need to create/extend models to cover all code paths. For example, I need to create one2many, many2one, many2many fields other than (parent_id/child_ids). Is it possible to create a module just for testing?

Also, I think there are several problems in the current implementation of "child_of" (and with my "parent_of" too as mirrors it). For example, The self-reference one2many case is not correct as it replaces the "left" column in the domain with "id" and indeed it gave me a wrong result in manual testing.

<blockquote cite="mid:CAP4GjTJ=v=P6YfhtytbvrHr+doa=E7+4vRN0VJ0tM=4DKz7YhQ@mail.gmail.com" type="cite">

I haven't thoroughly checked your implementation yet, but as an
initial remark I would find it logical that 'parent_of' mirrors the
use cases of 'child_of'. This means that it should support all models
with a `parent_id` or `_parent_name` column, with or without
`_parent_store` (parent_left/right).
For those without `_parent_store`, a WITH RECURSIVE implementation is
good, but for those with `_parent_store` set, a single query with
parent_left/parent_right is better/faster. A simple domain
[('parent_left', '=',
n.parent_right)] should give you all ancestors of node `n` (including
the `n` itself).
This would not work unless the leaf is the left-most node within its siblings.

Actually, a single query is possible but I do not think it can be implemented as a domain as it needs a conditional cross join. It should be something like this (not tested):

SELECT parent.id
FROM table AS child, table AS parent
WHERE child.parent_left BETWEEN parent.parent_left AND parent.parent_right
        AND child.id IN (id1, id2, ...)

While I think the PostgreSQL optimizer will intelligently handle the cross join, I do not think it will be faster than the recursive CTE. The main point of nested set model is to avoid traversing all branches down the tree to reach leafs. OTH, to reach parents, it is one way to the top.


<blockquote cite="mid:CAP4GjTJ=v=P6YfhtytbvrHr+doa=E7+4vRN0VJ0tM=4DKz7YhQ@mail.gmail.com" type="cite">

Thanks for your excellent work!

Olivier

_______________________________________________
Mailing-List: https://www.odoo.com/groups/community-framework-62
Post to: mailto:expert-framework@mail.odoo.com
Unsubscribe: https://www.odoo.com/groups?unsubscribe