> On 29 Sep 2020, at 18:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> [ Pulling Daniel into this older thread seems like the cleanest way to
> unify the two threads ]
>
> Masahiko Sawada <masahiko.sawada@2ndquadrant.com> writes:
>> If we have ALTER TABLE ONLY / DROP EXPRESSION update the attlocal
>> column of children to true to fix the issue you raised, my proposed
>> patch is not necessary. OTOH if we fix it by prohibiting the command,
>> I guess we need both patches to fix the issues.
>
> Right, Peter already mentioned that we need a further pg_dump fix on
> top of prohibiting the ONLY ... DROP EXPRESSION case.
>
> Bug #16622 [1] is another report of this same issue, and in that thread,
> Daniel argues that the right fix is just
>
> + /*
> + * Skip if the column isn't local to the table's definition as the attrdef
> + * will be printed with the inheritance parent table definition
> + */
> + if (!tbinfo->attislocal[adnum - 1])
> + return;
>
> without the attgenerated clause that Masahiko-san proposes.
> I think however that that's wrong. It is possible to have
> a non-attislocal column that has its own default, because
> you can do
Ah, that's the sequence I didn't think of. I agree with your assessment of
this being wrong. Thanks!
> This does not cause child2.f1's attislocal property to become
> true. Maybe it should have, but it's probably too late for
> that; at least, pg_dump couldn't assume it's true in older
> servers.
Do you recall the rationale for it not being set to true? I didn't spot
anything in the commit history. Intuitively it seems a tad odd.
> Therefore, since we can't tell whether the default
> is inherited or not, we'd better dump it.
Agreed.
> (I recall that pg_dump has a hack somewhere that checks for
> textual equality of CHECK conditions to avoid dumping redundant
> child copies. Maybe we could do something similar here.)
Unless someone beats me to it I will take a stab at this to see what it would
look like.
cheers ./daniel
