Re: [bug fix] ALTER TABLE SET LOGGED/UNLOGGED on a partitioned table does nothing silently

On Mon, Dec 7, 2020 at 11:36 AM tsunakawa(dot)takay(at)fujitsu(dot)com
<tsunakawa(dot)takay(at)fujitsu(dot)com> wrote:
>
> From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
> > IMHO, we should also change the parent table. Say, I have 2 local
> > partitions for a logged table, then I alter that table to
> > unlogged(with your patch, parent table doesn't become unlogged whereas
> > the partitions will), and I detach all the partitions for some reason.
> > Now, the user would think that he set the parent table to unlogged but
> > it didn't happen. So, I think we should also change the parent table's
> > logged/unlogged property though it may not have data associated with
> > it when it has all the partitions. Thoughts?
>
> I'd like to think that the logged/unlogged property is basically specific to each storage unit, which is a partition here, and ALTER TABLE on a partitioned table conveniently changes the properties of underlying storage units. (In that regard, it's unfortunate that it fails with an ERROR to try to change storage parameters like fillfactor with ALTER TABLE.)
>

Do you mean to say that if we detach all the partitions(assuming they
are all unlogged) then the parent table(assuming logged) gets changed
to unlogged? Does it happen on master? Am I missing something here?

>
> > I think we can add foreign partition case into postgres_fdw.sql so
> > that the tests will run as part of make check-world.
>
> I was hesitant to add this test in postgres_fdw, but it seems to be a good place considering that postgres_fdw, and other contrib modules as well, are part of Postgres core.
>

+1 to add tests in postgres_fdw.

>
> > How about documenting all these points in alter_table.sgml,
> > create_table.sgml and create_foreign_table.sgml under the partition
> > section?
>
> I'd like to add a statement in the description of ALTER TABLE SET LOGGED/UNLOGGED as other ALTER actions do. I don't want to add a new partition section for all CREATE/ALTER actions in this patch.
>

+1.

>
> If there's no objection, I think I'll submit the (hopefully final) revised patch after a few days.
>

Please do so. Thanks.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com