Re: DROP COLUMN misbehaviour with multiple inheritance

Hannu Krosing <hannu(at)tm(dot)ee> writes:
> I still think that this should be fixed in 7.3, but the inhcount
> attribute should show all tables where the column is defined, not just
> inherited. The default, no-inheritance case should set the column to 1.

Well, no, because then a locally defined column is indistinguishable
from a singly-inherited column, breaking the cases that the original
attisinherited patch was supposed to fix.

It doesn't fix the ONLY problem, either. Consider

create table p1 (f1 int);
create table p2 (f1 int);
create table c () inherits(p1, p2);
--c.f1 now has definition-count 2

drop ONLY column p1.f1;
--c.f1 now has count 1?
drop column p2.f1;
--c.f1 removed because count went to 0?

It might look like we could fix this by defining DROP ONLY as not
touching the child-table definition-counts at all; then a DROP ONLY
effectively makes a child column look like it's locally defined
instead of inherited. But that trick only works once. Consider:

create table p1 (f1 int);
create table p2 (f1 int);
create table c () inherits(p1, p2);
--c.f1 now has definition-count 2

drop ONLY column p1.f1;
--c.f1 still has count 2?
drop ONLY column p2.f1;
--c.f1 still has count 2?
drop column c.f1
--fails because count>1, so there is now no way to delete c.f1

I think we could make all these cases work if we replaced attisinherited
with *two* columns, a boolean attislocal(ly defined) and a count of
(direct) inheritances. DROP ONLY would have the effect of decrementing
the count and setting attislocal to true in each direct child; recursive
DROP would decrement the count and then drop if count is 0 *and*
attislocal is not set. At the start of a recursion, we'd allow DROP
only if count is 0 (and, presumably, attislocal is true, else the column
would not be there...).

Question is, is fixing these cases worth this much trouble? I think the
two-column solution is actually free in terms of storage space in
pg_attribute, because of alignment considerations. But it's still a
large reworking of the existing patch, and we have other fish to fry by
Sunday.

In any case I am inclined to reject the patch as-it-stands, because it
fixes one problem at the cost of introducing new ones.

regards, tom lane