Skip site navigation (1) Skip section navigation (2)

Re: [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>, Bernd Helmle <mailings(at)oopsware(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org, Thom Brown <thombrown(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns
Date: 2010-01-29 00:13:02
Message-ID: 4B62280E.8080000@ak.jp.nec.com (view raw or flat)
Thread:
Lists: pgsql-hackers
(2010/01/29 0:46), Robert Haas wrote:
> 2010/1/27 KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)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(at)ak(dot)jp(dot)nec(dot)com>

In response to

Responses

pgsql-hackers by date

Next:From: Robert HaasDate: 2010-01-29 00:29:27
Subject: Re: [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns
Previous:From: Ivan Sergio BorgonovoDate: 2010-01-29 00:00:26
Subject: returning array in a field together with other types

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group