Re: patch for check constraints using multiple inheritance

Robert Haas wrote:
> I don't think that this is much cleaner than the global variable
> solution; you haven't really localized that need to know about the new
> flag in any meaningful way, the hacks in ATOneLevelRecusion()
> basically destroy any pretense of that code possibly being reusable
> for some other caller. However, there's a more serious problem, which
> is that it doesn't in general fix the bug: try it with the
> top1/top2/bottom/basement example I posted upthread. If you add the
> same column to both top1 and top2 and then drop it in both top1 and
> top2, basement ends up with a leftover copy. The problem is that
> "only visit each child once" is not the right algorithm; what you need
> to do is "only visit the descendents of each child if no merge
> happened at the parent". I believe that the only way to do this
> correct is to merge the prep stage into the execution stage, as the
> code for adding constraints already does. At the prep stage, you
> don't have enough information to determine which relations you'll
> ultimately need to update, since you don't know where the merges will
> happen.
>
Hello Robert,

Again thanks for looking at the patch. Unfortunately I missed the
top1/top2 example earlier, but now I've seen it I understand that it is
impossible to fix this problem during the prep stage, without looking at
actual existing columns, i.e. without the merge code. Running the
top1/top2 example I'd also have expected an error while adding the
column to the second top, since the columns added to top1 and top2 are
from a different origin. It is apparently considered good behaviour,
however troubles emerge when e.g. trying to rename a_table_column in the
top1/top2 example, where that is no problem in the 'lollipop' structure,
that has a single origin.

ALTER TABLE top RENAME COLUMN a_table_column TO another_table_column;

SELECT t.oid, t.relname, a.attname, a.attinhcount
FROM pg_class t
JOIN pg_attribute a ON (a.attrelid = t.oid)
JOIN pg_namespace n ON (t.relnamespace = n.oid)
WHERE n.nspname = 'test_inheritance' AND a.attname LIKE '%table_column%'
ORDER BY oid;

I do not completely understand what you mean with the destruction of
reusability of ATOneLevelRecursion, currently only called by
ATPrepAddColumn - in the patch it is documented in the definition of
relVisited that is it visit info for each subcommand. The loop over
subcommands is in ATController, where the value is properly reset for
each all current and future subcommands. Hence the ATOneLevelRecursion
routing is usable in its current form for all callers during the prep
stage, and not only ATPrepAddColumn.
> I am of the opinion that the chances of a unifying solution popping up
> are pretty much zero. :-)
>
Me too, now I understand the fixes must be in the execution stages.

regards,
Yeb Havinga