Re: tracking inherited columns (was: patch for check constraints using multiple inheritance)

Robert Haas wrote:
> On Tue, Aug 3, 2010 at 3:05 PM, Yeb Havinga <yebhavinga@gmail.com> wrote:
>   
>> Yeb Havinga wrote:
>>     
>>> The underlying cause is the failure of the code to recognize that if
>>> relation C inherits from both A and B, where A and B both have column x,
>>> that A.x 'is the same as' B.x, where the 'is the same as' relation is the
>>> same that holds for (A.x, C.x) and (B.x, C.x), which the code does a lot of
>>> trouble for to recognize. This means that if some definition is altered on
>>> A.x, only C.x is updated and B.x not touched. IMO this is wrong and either a
>>> multiple inheritance structure like this should be prohibited, since the
>>> user did not explicitly declare that A.x and B.x 'are the same' (by e.g.
>>> defining a relation D.x and have A and B inherit from that), or the code
>>> should update parents of relations when the childs are updated.
>>>       
>> Thinking about this a bit more, the name 'is the same as' is a bit
>> confusing, since that relation might not be commutative. C.x 'inherits
>> properties from' A.x, or C.x 'is defined by' A.x are perhaps better names,
>> that reflect that the converse might not hold. OTOH, what does C.x 'inherits
>> (all) properties from' A.x mean? If it means that for all properties P,
>> P(C.x) iff P(A.x), then C.x =  A.x commutatively and by similar reasoning
>> A.x = B.x.
>>
>>     
>>> ALTER TABLE top1 RENAME COLUMN a_table_column TO another_table_column;
>>>       
>> When looking for previous discussions that was referred to upthread, the
>> first thing I found was this recent thread about the exactly the same
>> problem  http://archives.postgresql.org/pgsql-hackers/2010-01/msg03117.php
>>
>> Sorry for the double post, however the previous discussion postponed work to
>> .. now, so maybe there is some value in first trying to specify exactly what
>> 'inherits' means, and derive consequences for code behaviour from that.
>>     
>
> Yeah, I was thinking about that thread, too, on my drive home from
> Metuchen.
I just read that thread. In the beginning there is a short discussion 
what the non-astonishing behaviour of the RENAME in the case of multiple 
origin inheritance should be, which is preventing renames or any 
property change in that case. I think we should explore the possibilty 
of allowing the RENAME more.

What if on a RENAME of a column (maybe with a necessary explicit 
CASCADE) in the multiple origin parent case, the parents with the same 
columns are altered too? I don't think it is ashtonishing for users; 
after all they've created the tree in the first place, but mostly for 
programmers with some experience with inheritance in computer languages: 
inheritance should go down, not up. That's why I tried to make reasoning 
exact, to figure out why it would be ok (or not) to update another 
parent as well. The reasoning can be made more formal/exact, but I 
believe in its current form it makes a strong case to technically allow 
to prograpage property changes to other parents as well (if they have 
the same inherited column).
> 1. If you're changing properties of a column, you need to verify for
> each relation in the inheritance tree that the "expected attinhcount"
> and the actual attinhcount match.  If, for any relation in the
> inheritance tree rooted at the named table, they don't, then they are
> doubly inherited there, from some other table outside the hierarchy
> rooted at the named table, and the operation must fail.
>   
If we want to block these RENAMES, yes. This is essentially KaiGai's 
patch http://archives.postgresql.org/pgsql-hackers/2010-01/msg02878.php
> 2. If you're adding a column, you need to propagate the new column to
> relations that don't have it yet, but if you find one that already has
> it than you adjust attinhcount and don't recurse to its chidlren.
>   
Sound ok.
> 3. If you're dropping a column, you essentially decrement the
> attinhcount of all your children; then you recurse into any that reach
> attincount = 0 and not attislocal and drop the column there as well.
>   
This too.

regards,
Yeb Havinga