Re: BUG #5856: pg_attribute.attinhcount is not correct.

[moving to pgsql-hackers]

On Thu, Feb 03, 2011 at 11:24:42AM -0500, Robert Haas wrote:
> On Mon, Jan 31, 2011 at 6:42 AM, Naoya Anzai
> <anzai-naoya(at)mxu(dot)nes(dot)nec(dot)co(dot)jp> wrote:
> > In PostgreSQL8.4.5, I found that the catalog pg_attribute.attinhcount is not
> > correct.
> >
> > I executed the following queries.
> >
> > --------------------------------------------------------------------------
> > create table "3_grandchild"();
> > create table "2_child"();
> > create table "1_parent"();
> >
> > alter table "3_grandchild" inherit "2_child";
> > alter table "2_child" inherit "1_parent";
> >
> > alter table "3_grandchild" add column c1 text;
> > alter table "2_child" add column c1 text;
> > alter table "1_parent" add column c1 text;
> >
> > select c.relname,a.attname,a.attinhcount from pg_attribute a,pg_class c
> > where a.attrelid=c.oid
> > and relname in ('1_parent','2_child','3_grandchild')
> > and attname not in('xmax','xmin','cmin','cmax','tableoid','ctid')
> > order by relname;
> >
> > ? ?relname ? ?| attname | attinhcount
> > ?--------------+---------+-------------
> > ?1_parent ? ? | c1 ? ? ?| ? ? ? ? ? 0
> > ?2_child ? ? ?| c1 ? ? ?| ? ? ? ? ? 1
> > ?3_grandchild | c1 ? ? ?| ? ? ? ? ? 2
> > ?(3 rows)
> > --------------------------------------------------------------------------
> >
> > "3_grandchild"'s attinhcount should be 1.
>
> I think this is a manifestation the same problem mentioned here:
>
> http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=31b6fc06d83c6de3644c8f2921eb7de0eb92fac3
>
> I believe this requires some refactoring to fix. It would be good to do that.

The best way I can see is to make ATExecAddColumn more like ATExecDropColumn,
ATAddCheckConstraint, and ATExecDropConstraint. Namely, recurse at Exec-time
rather than Prep-time, and cease recursing when we satisfy the ADD COLUMN with a
merge. Did you have something else in mind?

Incidentally, when we satisfy an ADD COLUMN with a merge, we do not check or
update attnotnull:

create table parent();
create table child(c1 text) inherits (parent);
alter table parent add column c1 text not null;
\d child

We could either update attnotnull (and schedule a phase-3 scan of the table) or
throw an error. For ALTER TABLE ... INHERIT, we throw the error. For CREATE
TABLE ... INHERITS, we add the NOT NULL (and no scan is needed). I'd weakly
lean toward throwing the error. Opinions?

nm