Re: [PATCH v1] Fix for bug #19773 : pg_dump differing generation expressions of inherited columns

Hi Henson,

I looked further into this. I considered a few approaches to resolve
these edge cases.

> The
> "Indirect loop" repair picks the pair (b, b.g). b.g is already
> separate and there is no b -> b.g edge to remove, so it only adds
> another b.g -> b edge and the same loop is found again.

1. One idea I tried was to change the loop repair logic to handle 1 &
2a. This would check the edge direction before picking the edge to
remove. And therefore b.g -> b would would not be picked. Similarly I
also added a new loop repair rule for 2a, for loops that go through
the child -> parent-expression edge.. But I decided against this route
because it seemed likely that more edge cases would need to be found
and fixed. And it would become difficult to maintain.

2. I considered always dumping the child's 'alter ... set expression'
at the end. This would dump them irrespective of whether they matched
the parent or not and avoid any loops. This is similar to how 'set
default' is currently handled for child columns. But this would have
broken the restore to v16 and below as 'set expression' doesn't exist
in the older versions.

3. I decided to avoid adding new dependencies and instead separately
check which child tables needed their own 'alter ... set expression'.
I kept some of the code similar to what I had before. If the child has
a different expression from the parent, its 'alter ... set expression'
is printed anyway because 'separate' is set to true. If a child and
parent have the same expression but that happened because the parent's
own expression is dumped as a separate ALTER, instead of creating an
edge for it, the child would be created before that ALTER runs and
inherit the old expression. Instead of creating an edge, we check it
separately after the tables have been sorted. This seemed the cleanest
approach.

In this, I have created a new function flagInhGenerationExprs which
checks for the cases where a parent has the separate flag set and is
actually dumped. It's run after sorting the tables. I have created a
new field in AttrDefInfo called
maybe_dump_later_if_altered_expression. This is set in flagInhAttrs()
for the cases which might need this special check later. This fixes 1,
2a, and 2b from your cases.

Note this still fails for the following multi-parent scenario.

CREATE TABLE gp (id int, v int, gs int GENERATED ALWAYS AS (v * 3) STORED);
CREATE TABLE p1 () INHERITS (gp);
ALTER TABLE p1 ALTER COLUMN gs SET EXPRESSION AS (v * 7); -- p1: (v *
7), inherited-only
CREATE TABLE p2 (id int, v int, gs int GENERATED ALWAYS AS (v * 7) STORED);
CREATE TABLE c () INHERITS (p1, p2); -- parents agree: both (v * 7)
INSERT INTO c VALUES (1, 1);

This scenario also fails on master and defaults seem to have the same
limitation there. So I haven't tried to fix with any other special
case. Let me know what you think of this. It worked with the first
approach of handling edge cases in loop repair but I thought that
might be risky.

I'm attaching the patch and have added this to commitfest
(https://commitfest.postgresql.org/patch/7519/) Please take a look.

> Since this happens on master without the patch, I think it is
> better handled as a separate backend bug, apart from this patch.

Regarding this item, are you planning to work on this? If not, I am
happy to take it up.

Thanks & Regards,
Nitin Motiani
Google