On 2021-01-29 17:41, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> I've had another go at this, and I've found a solution that appears to
>> address all the issues I'm aware of. It's all very similar to the
>> previously discussed patches. The main difference is that previous
>> patches had attempted to use something like tbinfo->attislocal to
>> determine whether a column was inherited, but that's not correct. This
>> patch uses the existing logic in flagInhAttrs() to find whether there is
>> a matching parent column with a generation expression. I've added
>> pg_dump test cases here to check the different variations that the code
>> addresses.
>
> This is a clear improvement on the current situation, and given that
> this issue is over a year old, I think we should push and back-patch
> this in time for February's releases.
done
I will continue working on the other issues that we have been discussing.
> However ... this doesn't solve all the cases noted in this thread.
> In the first example I gave at [1],
>
> d3=# create table parent (f1 int default 2);
> CREATE TABLE
> d3=# create table child (f1 int default 3) inherits(parent);
> NOTICE: merging column "f1" with inherited definition
> CREATE TABLE
> d3=# create table child2() inherits(parent);
> CREATE TABLE
> d3=# alter table child2 alter column f1 set default 42;
> ALTER TABLE
>
> pg_dump still fails to restore child2.f1's non-inherited default.
> That's probably a pre-existing problem, since it doesn't involve
> GENERATED at all, but we shouldn't forget about it.
>
> Also, in the example from [2],
>
> d3=# create table pp1 (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
> CREATE TABLE
> d3=# create table cc1 (a int, b int GENERATED ALWAYS AS (a * 3) STORED);
> CREATE TABLE
> d3=# alter table cc1 inherit pp1;
> ALTER TABLE
>
> pg_dump now omits to dump cc1's generation expression, which seems
> strictly worse than before. Admittedly, the backend likely ought to
> be rejecting this scenario, but it doesn't do so today.
>
> Neither of these points seem like a reason to reject this patch,
> they're just adjacent work that remains to be done.
>
> regards, tom lane
>
> [1] https://www.postgresql.org/message-id/660925.1601397436%40sss.pgh.pa.us
> [2] https://www.postgresql.org/message-id/661371.1601398006%40sss.pgh.pa.us
>
>
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/