Re: pg_restore - generated column - not populating

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: pg_restore - generated column - not populating

Tom Lane-2
[ redirecting to pgsql-bugs ]

Santosh Udupi <[hidden email]> writes:
> Here is my table structure.

Indeed, this looks pretty busted, both in v13 and HEAD.  It seems that
pg_dump is not coping well with GENERATED columns attached to a
partition parent table.  I made the attached script with a bit of
sample data, loaded it into an empty database, and dumped it.
The dump is evidently assuming that ALTER TABLE ATTACH PARTITION
is going to cause the generated-ness of the columns to propagate
to the children, but it doesn't.  There also seems to be considerable
confusion about which columns of the child tables should be included
in the dumped data.

I suspect this example is revealing bugs in both the backend
(ATTACH PARTITION ought to take care of this, no?) and pg_dump
(the backend can't be blamed for pg_dump's choices of columns
to dump).  Peter?

                        regards, tom lane


create table tbl_main(

item_id int GENERATED ALWAYS AS IDENTITY,
-----------------------------------------------------
operating_offices int [] GENERATED ALWAYS AS (
nullif(array[(info->>'o')::int], '{NULL}') ) stored ,
-----------------------------------------------------
primary_bill_to_id int   GENERATED ALWAYS as ((info->>'vp')::int) stored ,
----------------------------------------------
item_status_array text [] GENERATED ALWAYS as ( array[
coalesce(info->>'qr', info->>'s'), info->>'v'] ) stored ,
-------------------------------------------------
info jsonb
------------------------------
,is_complete bool  GENERATED ALWAYS as (coalesce( (info->>'lf')::bool =
true or (info->>'lg')::bool = true, false)) stored
--------------------------------------------
,is_deleted bool GENERATED ALWAYS as ( coalesce( (info->>'cv')::bool,
false) ) stored
------------------------------
,is_a_template bool GENERATED ALWAYS as ( coalesce( (info->>'cw')::bool,
false) ) stored
-------------------------------------------
,created_by_user_id int
,created_on timestamptz default now()
----------------------------------
,primary key(item_id,created_on )


) partition by range (created_on) ;

---=================================================================
-- *** index

CREATE INDEX tbl_main_idxgin ON tbl_main USING gin (info);


---=================================================================
 -- **** partitions

-- default partition
create table tbl_main_partition_default
partition of tbl_main default;

create table tbl_main_partition_2021
partition of tbl_main
for values from ('2020-01-01') to ('2022-01-01');

create table tbl_main_partition_2022
partition of tbl_main
for values from ('2022-01-01') to ('2023-01-01');

create table tbl_main_partition_2023
partition of tbl_main
for values from ('2023-01-01') to ('2024-01-01');

insert into tbl_main (info, created_on) values
 ('{"vp": 42, "o": 1000, "qr": "foo"}', '2023-06-01'),
 ('{"vp": 43, "o": 1001, "qr": "bar"}', '2013-06-01');
Reply | Threaded
Open this post in threaded view
|

Re: pg_restore - generated column - not populating

Santosh Udupi
Hi Tom, To add to your finding: we compared between the postgresql pg_dump and the pgAdmin (win) pg_dump. The main table definition remains the same but the partition table versions differ


image.png


image.png


On Tue, Feb 23, 2021 at 5:03 PM Tom Lane <[hidden email]> wrote:
[ redirecting to pgsql-bugs ]

Santosh Udupi <[hidden email]> writes:
> Here is my table structure.

Indeed, this looks pretty busted, both in v13 and HEAD.  It seems that
pg_dump is not coping well with GENERATED columns attached to a
partition parent table.  I made the attached script with a bit of
sample data, loaded it into an empty database, and dumped it.
The dump is evidently assuming that ALTER TABLE ATTACH PARTITION
is going to cause the generated-ness of the columns to propagate
to the children, but it doesn't.  There also seems to be considerable
confusion about which columns of the child tables should be included
in the dumped data.

I suspect this example is revealing bugs in both the backend
(ATTACH PARTITION ought to take care of this, no?) and pg_dump
(the backend can't be blamed for pg_dump's choices of columns
to dump).  Peter?

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: pg_restore - generated column - not populating

Peter Eisentraut-7
In reply to this post by Tom Lane-2
On 24.02.21 02:03, Tom Lane wrote:

> [ redirecting to pgsql-bugs ]
>
> Santosh Udupi <[hidden email]> writes:
>> Here is my table structure.
>
> Indeed, this looks pretty busted, both in v13 and HEAD.  It seems that
> pg_dump is not coping well with GENERATED columns attached to a
> partition parent table.  I made the attached script with a bit of
> sample data, loaded it into an empty database, and dumped it.
> The dump is evidently assuming that ALTER TABLE ATTACH PARTITION
> is going to cause the generated-ness of the columns to propagate
> to the children, but it doesn't.  There also seems to be considerable
> confusion about which columns of the child tables should be included
> in the dumped data.
>
> I suspect this example is revealing bugs in both the backend
> (ATTACH PARTITION ought to take care of this, no?) and pg_dump
> (the backend can't be blamed for pg_dump's choices of columns
> to dump).  Peter?

The backend side of this would be fixed by the proposed
<https://www.postgresql.org/message-id/ac35da1c-e746-ea19-bfc3-84819a4e907d%40enterprisedb.com>
(it's the same code for ALTER TABLE ... INHERIT and ATTACH PARTITION).

The pg_dump side can apparently be fixed by adding

diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 1a261a5545..c210883ca3 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -585,7 +585,7 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo,
int numTables)
             }

             /* Remove generation expression from child */
-           if (foundGenerated && !dopt->binary_upgrade)
+           if (foundGenerated && !dopt->binary_upgrade &&
!tbinfo->ispartition)
                 tbinfo->attrdefs[j] = NULL;
         }
     }

Looks like this was accidentally broken by the last minor release's
fixes in this area.


Reply | Threaded
Open this post in threaded view
|

Re: pg_restore - generated column - not populating

Peter Eisentraut-7
On 26.04.21 15:40, Peter Eisentraut wrote:

>> I suspect this example is revealing bugs in both the backend
>> (ATTACH PARTITION ought to take care of this, no?) and pg_dump
>> (the backend can't be blamed for pg_dump's choices of columns
>> to dump).  Peter?
>
> The backend side of this would be fixed by the proposed
> <https://www.postgresql.org/message-id/ac35da1c-e746-ea19-bfc3-84819a4e907d%40enterprisedb.com>
> (it's the same code for ALTER TABLE ... INHERIT and ATTACH PARTITION).
>
> The pg_dump side can apparently be fixed by adding
>
> diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
> index 1a261a5545..c210883ca3 100644
> --- a/src/bin/pg_dump/common.c
> +++ b/src/bin/pg_dump/common.c
> @@ -585,7 +585,7 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo,
> int numTables)
>             }
>
>             /* Remove generation expression from child */
> -           if (foundGenerated && !dopt->binary_upgrade)
> +           if (foundGenerated && !dopt->binary_upgrade &&
> !tbinfo->ispartition)
>                 tbinfo->attrdefs[j] = NULL;
>         }
>     }
>
> Looks like this was accidentally broken by the last minor release's
> fixes in this area.

Both of these issues have been fixed and will be in the next minor releases.


Reply | Threaded
Open this post in threaded view
|

Re: pg_restore - generated column - not populating

Santosh Udupi
Thank you so much!!

On Tue, May 4, 2021 at 5:26 AM Peter Eisentraut <[hidden email]> wrote:
On 26.04.21 15:40, Peter Eisentraut wrote:
>> I suspect this example is revealing bugs in both the backend
>> (ATTACH PARTITION ought to take care of this, no?) and pg_dump
>> (the backend can't be blamed for pg_dump's choices of columns
>> to dump).  Peter?
>
> The backend side of this would be fixed by the proposed
> <https://www.postgresql.org/message-id/ac35da1c-e746-ea19-bfc3-84819a4e907d%40enterprisedb.com>
> (it's the same code for ALTER TABLE ... INHERIT and ATTACH PARTITION).
>
> The pg_dump side can apparently be fixed by adding
>
> diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
> index 1a261a5545..c210883ca3 100644
> --- a/src/bin/pg_dump/common.c
> +++ b/src/bin/pg_dump/common.c
> @@ -585,7 +585,7 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo,
> int numTables)
>             }
>
>             /* Remove generation expression from child */
> -           if (foundGenerated && !dopt->binary_upgrade)
> +           if (foundGenerated && !dopt->binary_upgrade &&
> !tbinfo->ispartition)
>                 tbinfo->attrdefs[j] = NULL;
>         }
>     }
>
> Looks like this was accidentally broken by the last minor release's
> fixes in this area.

Both of these issues have been fixed and will be in the next minor releases.