> On 29 Sep 2020, at 18:37, Tom Lane <t...@sss.pgh.pa.us> wrote: > > [ Pulling Daniel into this older thread seems like the cleanest way to > unify the two threads ] > > Masahiko Sawada <masahiko.saw...@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
