(2010/01/28 6:58), Robert Haas wrote: > On Wed, Jan 27, 2010 at 3:42 PM, Robert Haas<robertmh...@gmail.com> wrote: >> On Wed, Jan 27, 2010 at 10:17 AM, KaiGai Kohei<kai...@kaigai.gr.jp> wrote: >>> (2010/01/27 23:29), Robert Haas wrote: >>>> >>>> 2010/1/27 KaiGai Kohei<kai...@ak.jp.nec.com>: >>>>> >>>>> The attached patch is revised one based on the V3 approach. >>>>> The only difference from V3 is that it also applies checks on the >>>>> AT_AlterColumnType option, not only renameatt(). >>>> >>>> I think I was clear about what the next step was for this patch in my >>>> previous email, but let me try again. >>>> >>>> http://archives.postgresql.org/pgsql-hackers/2010-01/msg02407.php >>>> >>>> See also Tom's comments here: >>>> >>>> http://archives.postgresql.org/pgsql-hackers/2010-01/msg00110.php >>>> >>>> I don't believe that either Tom or I are prepared to commit a patch >>>> based on this approach, at least not unless someone makes an attempt >>>> to do it the other way and finds an even more serious problem. If >>>> you're not interested in rewriting the patch along the lines Tom >>>> suggested, then we should just mark this as Returned with Feedback and >>>> move on. >>> >>> The V3/V5 patch was the rewritten one based on the Tom's comment, as is. >>> It counts the expected inhcount at the first find_all_inheritors() time >>> at once, and it compares the pg_attribute.attinhcount. >>> (In actually, find_all_inheritors() does not have a capability to count >>> the number of merged from a common origin, so I newly defined the >>> find_all_inheritors_with_inhcount().) >>> >>> Am I missing something? >> >> Err... I'm not sure. I thought I understood what the different >> versions of this patch were doing, but apparently I'm all confused. >> I'll take another look at this. > > OK, I took a look at this. It's busted: > > test=# create table top (a integer); > CREATE TABLE > test=# create table upper1 () inherits (top); > CREATE TABLE > test=# create table upper2 () inherits (top); > CREATE TABLE > test=# create table lower1 () inherits (upper1, upper2); > NOTICE: merging multiple inherited definitions of column "a" > CREATE TABLE > test=# create table lower2 () inherits (upper1, upper2); > NOTICE: merging multiple inherited definitions of column "a" > CREATE TABLE > test=# create table spoiler (a integer); > CREATE TABLE > test=# create table bottom () inherits (lower1, lower2, spoiler); > NOTICE: merging multiple inherited definitions of column "a" > NOTICE: merging multiple inherited definitions of column "a" > CREATE TABLE > test=# alter table top rename a to b; > ALTER TABLE > test=# select * from spoiler; > ERROR: could not find inherited attribute "a" of relation "bottom"
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? Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kai...@ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
