(2010/01/29 0:46), Robert Haas wrote:
> 2010/1/27 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> Hmm, indeed, this logic (V3/V5) is busted.
>>
>> The idea of V4 patch can also handle this case correctly, although it
>> is lesser in performance.
>> I wonder whether it is really unacceptable cost in performance, or not.
>> Basically, I assume ALTER TABLE RENAME/TYPE is not frequent operations,
>> and I don't think this bugfix will damage to the reputation of PostgreSQL.
>>
>> Where should we go on the next?
>
> Isn't the problem here just that the following comment is 100% wrong?
>
> /*
> * Unlike find_all_inheritors(), we need to walk on
> child relations
> * that have diamond inheritance tree, because this
> function has to
> * return correct expected inhecount to the caller.
> */
>
> It seems to me that the right solution here is to just add one more
> argument to find_all_inheritors(), something like List
> **expected_inh_count.
>
> Am I missing something?
The find_all_inheritors() does not walk on child relations more than
two times, even if a child has multiple parents inherited from common
origin, because list_concat_unique_oid() ignores the given OID if it
is already on the list. It means all the child relations under the
relation already walked on does not checked anywhere. (Of course,
this assumption is correct for the purpose of find_all_inheritors()
with minimum cost.)
What we want to do here is to compute the number of times a certain
child relation is inherited from a common origin; it shall be the
expected-inhcount. So, we need an arrangement to the logic.
For example, see the following diagram.
T2 / \
T1 T4---T5 \ / T3
If we call find_all_inheritors() with T1. The find_inheritance_children()
returns T2 and T3 for T1.
Then, it calls find_inheritance_children() for T2, and it returns T4.
Then, it calls find_inheritance_children() for T3, and it returns T4, but
it is already in the "rels_list", so list_concat_unique_oid() ignores it.
Then, it calls find_inheritance_children() for T4, and it returns T5.
In this example, we want the expected inhcount for T2 and T3 should be 1,
for T4 and T5 should be 2. However, it walks on T4 and T5 only once, so
they will have 1 incorrectly.
Even if we count up the ignored OID (T4), find_all_inheritors() does not
walk on T5, because it is already walked on obviously when T4 is ignored.
However, my V3/V5 logic is also busted when a certain relation is inherited
from a relation which has multiple parents.
Right now, we have only the V4 logic which works correctly....
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>