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

On Wed, Dec 2, 2020 at 1:52 PM tsunakawa.takay@fujitsu.com
<tsunakawa.takay@fujitsu.com> wrote:

ALTER TABLE SET LOGGED/UNLOGED on a partitioned table completes successfully, but nothing changes.

Yeah, true.

I expected the underlying partitions are changed accordingly. The attached patch fixes this.

My thoughts:
1) What happens if a partitioned table has a foreign partition along
with few other local partitions[1]? Currently, if we try to set
logged/unlogged of a foreign table, then an "ERROR: "XXXX" is not a
table" is thrown. This makes sense. With your patch also we see the
same error, but I'm not quite sure, whether it is setting the parent
and local partitions to logged/unlogged and then throwing the error?
Or do we want the parent relation and the all the local partitions
become logged/unlogged and give a warning saying foreign table can not
be made logged/unlogged?
2) What is the order in which the parent table and the partitions are
made logged/unlogged? Is it that first the parent table and then all
the partitions? What if an error occurs as in above point for a
foreign partition or a partition having foreign key reference to a
logged table? During the rollback of the txn, will we undo the setting
logged/unlogged?
3) Say, I have two logged tables t1 and t2. I altered t1 to be
unlogged, and then I attach logged table t2 as a partition to t1, then
what's the expectation? While attaching the partition, should we also
make t2 as unlogged?
4) Currently, if we try to set logged/unlogged of a foreign table,
then an "ERROR: "XXXX" is not a table" is thrown. I also see that, in
general ATWrongRelkindError() throws an error saying the given
relation is not of expected types, but it doesn't say what is the
given relation kind. Should we improve ATWrongRelkindError() by
passing the actual relation type along with the allowed relation types
to show a bit more informative error message, something like "ERROR:
"XXXX" is a foreign table" with a hint "Allowed relation types are
table, view, index."
5) Coming to the patch, it is missing to add test cases.

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

On Wed, Dec 2, 2020 at 3:57 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

On Wed, Dec 2, 2020 at 1:52 PM tsunakawa.takay@fujitsu.com
<tsunakawa.takay@fujitsu.com> wrote:

ALTER TABLE SET LOGGED/UNLOGED on a partitioned table completes successfully, but nothing changes.

Yeah, true.

I expected the underlying partitions are changed accordingly. The attached patch fixes this.

My thoughts:
1) What happens if a partitioned table has a foreign partition along
with few other local partitions[1]? Currently, if we try to set
logged/unlogged of a foreign table, then an "ERROR: "XXXX" is not a
table" is thrown. This makes sense. With your patch also we see the
same error, but I'm not quite sure, whether it is setting the parent
and local partitions to logged/unlogged and then throwing the error?
Or do we want the parent relation and the all the local partitions
become logged/unlogged and give a warning saying foreign table can not
be made logged/unlogged?
2) What is the order in which the parent table and the partitions are
made logged/unlogged? Is it that first the parent table and then all
the partitions? What if an error occurs as in above point for a
foreign partition or a partition having foreign key reference to a
logged table? During the rollback of the txn, will we undo the setting
logged/unlogged?
3) Say, I have two logged tables t1 and t2. I altered t1 to be
unlogged, and then I attach logged table t2 as a partition to t1, then
what's the expectation? While attaching the partition, should we also
make t2 as unlogged?
4) Currently, if we try to set logged/unlogged of a foreign table,
then an "ERROR: "XXXX" is not a table" is thrown. I also see that, in
general ATWrongRelkindError() throws an error saying the given
relation is not of expected types, but it doesn't say what is the
given relation kind. Should we improve ATWrongRelkindError() by
passing the actual relation type along with the allowed relation types
to show a bit more informative error message, something like "ERROR:
"XXXX" is a foreign table" with a hint "Allowed relation types are
table, view, index."
5) Coming to the patch, it is missing to add test cases.

Sorry, I missed to add the foreign partition use case, specified as
[1]: CREATE TABLE tbl1 ( a INT, b INT, c TEXT default 'stuff', d TEXT, e TEXT ) PARTITION BY LIST (b);

[1]: CREATE TABLE tbl1 ( a INT, b INT, c TEXT default 'stuff', d TEXT, e TEXT ) PARTITION BY LIST (b);
CREATE TABLE tbl1 (
a INT,
b INT,
c TEXT default 'stuff',
d TEXT,
e TEXT
) PARTITION BY LIST (b);

CREATE FOREIGN TABLE foreign_part_tbl1_1(c TEXT, b INT, a INT, e TEXT,
d TEXT) SERVER loopback;
CREATE TABLE part_tbl1_2 (a INT, c TEXT, b INT, d TEXT, e TEXT);

ALTER TABLE tbl1 ATTACH PARTITION foreign_part_tbl1_1 FOR VALUES IN(1);
ALTER TABLE tbl1 ATTACH PARTITION part_tbl1_2 FOR VALUES IN(2);

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

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>

1) What happens if a partitioned table has a foreign partition along
with few other local partitions[1]? Currently, if we try to set
logged/unlogged of a foreign table, then an "ERROR: "XXXX" is not a
table" is thrown. This makes sense. With your patch also we see the
same error, but I'm not quite sure, whether it is setting the parent
and local partitions to logged/unlogged and then throwing the error?
Or do we want the parent relation and the all the local partitions
become logged/unlogged and give a warning saying foreign table can not
be made logged/unlogged?

Good point, thanks. I think the foreign partitions should be ignored, otherwise the user would have to ALTER each local partition manually or detach the foreign partitions temporarily. Done like that.

2) What is the order in which the parent table and the partitions are
made logged/unlogged? Is it that first the parent table and then all
the partitions? What if an error occurs as in above point for a
foreign partition or a partition having foreign key reference to a
logged table? During the rollback of the txn, will we undo the setting
logged/unlogged?

The parent is not changed because it does not have storage.
If some partition has undesirable foreign key relationship, the entire ALTER command fails. All the effects are undone when the transaction rolls back.

3) Say, I have two logged tables t1 and t2. I altered t1 to be
unlogged, and then I attach logged table t2 as a partition to t1, then
what's the expectation? While attaching the partition, should we also
make t2 as unlogged?

The attached partition retains its property.

4) Currently, if we try to set logged/unlogged of a foreign table,
then an "ERROR: "XXXX" is not a table" is thrown. I also see that, in
general ATWrongRelkindError() throws an error saying the given
relation is not of expected types, but it doesn't say what is the
given relation kind. Should we improve ATWrongRelkindError() by
passing the actual relation type along with the allowed relation types
to show a bit more informative error message, something like "ERROR:
"XXXX" is a foreign table" with a hint "Allowed relation types are
table, view, index."

Ah, maybe that's a bit more friendly. But I don't think it's worth bothering to mess ATWrongRelkindError() with a long switch statement to map a relation kind to its string representation. Anyway, I'd like it to be a separate topic.

5) Coming to the patch, it is missing to add test cases.

Yes, added in the revised patch.

I added this to the next CF.

Regards
Takayuki Tsunakawa

On Fri, Dec 4, 2020 at 8:22 AM tsunakawa.takay@fujitsu.com
<tsunakawa.takay@fujitsu.com> wrote:

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>

1) What happens if a partitioned table has a foreign partition along
with few other local partitions[1]? Currently, if we try to set
logged/unlogged of a foreign table, then an "ERROR: "XXXX" is not a
table" is thrown. This makes sense. With your patch also we see the
same error, but I'm not quite sure, whether it is setting the parent
and local partitions to logged/unlogged and then throwing the error?
Or do we want the parent relation and the all the local partitions
become logged/unlogged and give a warning saying foreign table can not
be made logged/unlogged?

Good point, thanks. I think the foreign partitions should be ignored, otherwise the user would have to ALTER each local partition manually or detach the foreign partitions temporarily. Done like that.

2) What is the order in which the parent table and the partitions are
made logged/unlogged? Is it that first the parent table and then all
the partitions? What if an error occurs as in above point for a
foreign partition or a partition having foreign key reference to a
logged table? During the rollback of the txn, will we undo the setting
logged/unlogged?

The parent is not changed because it does not have storage.
If some partition has undesirable foreign key relationship, the entire ALTER command fails. All the effects are undone when the transaction rolls back.

3) Say, I have two logged tables t1 and t2. I altered t1 to be
unlogged, and then I attach logged table t2 as a partition to t1, then
what's the expectation? While attaching the partition, should we also
make t2 as unlogged?

The attached partition retains its property.

4) Currently, if we try to set logged/unlogged of a foreign table,
then an "ERROR: "XXXX" is not a table" is thrown. I also see that, in
general ATWrongRelkindError() throws an error saying the given
relation is not of expected types, but it doesn't say what is the
given relation kind. Should we improve ATWrongRelkindError() by
passing the actual relation type along with the allowed relation types
to show a bit more informative error message, something like "ERROR:
"XXXX" is a foreign table" with a hint "Allowed relation types are
table, view, index."

Ah, maybe that's a bit more friendly. But I don't think it's worth bothering to mess ATWrongRelkindError() with a long switch statement to map a relation kind to its string representation. Anyway, I'd like it to be a separate topic.

5) Coming to the patch, it is missing to add test cases.

Yes, added in the revised patch.

I added this to the next CF.

Thanks! I will review the v2 patch and provide my thoughts.

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

On Fri, Dec 4, 2020 at 8:22 AM tsunakawa.takay@fujitsu.com
<tsunakawa.takay@fujitsu.com> wrote:

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>

1) What happens if a partitioned table has a foreign partition along
with few other local partitions[1]? Currently, if we try to set
logged/unlogged of a foreign table, then an "ERROR: "XXXX" is not a
table" is thrown. This makes sense. With your patch also we see the
same error, but I'm not quite sure, whether it is setting the parent
and local partitions to logged/unlogged and then throwing the error?
Or do we want the parent relation and the all the local partitions
become logged/unlogged and give a warning saying foreign table can not
be made logged/unlogged?

Good point, thanks. I think the foreign partitions should be ignored, otherwise the user would have to ALTER each local partition manually or detach the foreign partitions temporarily. Done like that.

Agreed.

2) What is the order in which the parent table and the partitions are
made logged/unlogged? Is it that first the parent table and then all
the partitions? What if an error occurs as in above point for a
foreign partition or a partition having foreign key reference to a
logged table? During the rollback of the txn, will we undo the setting
logged/unlogged?

The parent is not changed because it does not have storage.

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?

If some partition has undesirable foreign key relationship, the entire ALTER command fails. All the effects are undone when the transaction rolls back.

Hmm.

3) Say, I have two logged tables t1 and t2. I altered t1 to be
unlogged, and then I attach logged table t2 as a partition to t1, then
what's the expectation? While attaching the partition, should we also
make t2 as unlogged?

The attached partition retains its property.

This may lead to a situation where a partition table has few
partitions as logged and few as unlogged. Will it lead to some
problems? I'm not quite sure though. I feel while attaching a
partition to a table, we have to check whether the parent is
logged/unlogged and set the partition accordingly. While detaching a
partition we do not need to do anything, just retain the existing
state. I read this from the docs "Any indexes created on an unlogged
table are automatically unlogged as well". So, on the similar lines we
also should automatically make partitions logged/unlogged based on the
parent table. We may have to also think of cases where there exists a
multi level parent child relationship i.e. the table to which a
partition is being attached may be a child to another parent table.
Thoughts?

5) Coming to the patch, it is missing to add test cases.

Yes, added in the revised patch.

I think we can add foreign partition case into postgres_fdw.sql so
that the tests will run as part of make check-world.

How about documenting all these points in alter_table.sgml,
create_table.sgml and create_foreign_table.sgml under the partition
section?

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

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.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.)

This may lead to a situation where a partition table has few
partitions as logged and few as unlogged. Will it lead to some
problems?

No, I don't see any problem.

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.

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.

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

Regards
Takayuki Tsunakawa

On Mon, Dec 7, 2020 at 11:36 AM tsunakawa.takay@fujitsu.com
<tsunakawa.takay@fujitsu.com> wrote:

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.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

Does "ALTER TABLE ONLY parent" work correctly? Namely, do not affect
existing partitions, but cause future partitions to acquire the new
setting.

This sounds very much related to previous discussion on REPLICA IDENTITY
not propagating to partitions; see
/messages/by-id/201902041630.gpadougzab7v@alvherre.pgsql
particularly Robert Haas' comments at
/messages/by-id/CA+TgmoYjksObOzY8b1U1=BsP_m+702eOf42fqRtQTYio1NunbA@mail.gmail.com
and downstream discussion from there.

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>

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?

No, the parent remains logged in that case both on master and with patched. I understand this behavior is based on the idea that (1) each storage unit (=partition) is independent, and (2) a partitioned table has no storage so the logged/unlogged setting has no meaning. (I don't know there was actually such an idea and the feature was implemented on that idea, though.)

Regards
Takayuki Tsunakawa

From: Alvaro Herrera <alvherre@alvh.no-ip.org>

Does "ALTER TABLE ONLY parent" work correctly? Namely, do not affect
existing partitions, but cause future partitions to acquire the new
setting.

Yes, it works correctly in the sense that ALTER TABLE ONLY on a partitioned table does nothing because it has no storage and therefore logged/unlogged has no meaning.

This sounds very much related to previous discussion on REPLICA IDENTITY
not propagating to partitions; see
/messages/by-id/201902041630.gpadougzab7v@alvherre.pgsql
particularly Robert Haas' comments at
/messages/by-id/CA+TgmoYjksObOzY8b1U1=BsP_m+702eOf42fqRtQTYio
1NunbA@mail.gmail.com
and downstream discussion from there.

There was a hot discussion. I've read through it. I hope I'm not doing strange in light of that discussioin.

Regards
Takayuki Tsunakawa

On 2020-Dec-08, tsunakawa.takay@fujitsu.com wrote:

From: Alvaro Herrera <alvherre@alvh.no-ip.org>

Does "ALTER TABLE ONLY parent" work correctly? Namely, do not affect
existing partitions, but cause future partitions to acquire the new
setting.

Yes, it works correctly in the sense that ALTER TABLE ONLY on a
partitioned table does nothing because it has no storage and therefore
logged/unlogged has no meaning.

But what happens when you create another partition after you change the
"loggedness" of the partitioned table?

This sounds very much related to previous discussion on REPLICA IDENTITY
not propagating to partitions; see
/messages/by-id/201902041630.gpadougzab7v@alvherre.pgsql
particularly Robert Haas' comments at
/messages/by-id/CA+TgmoYjksObOzY8b1U1=BsP_m+702eOf42fqRtQTYio
1NunbA@mail.gmail.com
and downstream discussion from there.

There was a hot discussion. I've read through it. I hope I'm not
doing strange in light of that discussioin.

Well, my main take from that is we should strive to change all
subcommands together, in some principled way, rather than change only
one or some, in arbitrary ways.

From: Alvaro Herrera <alvherre@alvh.no-ip.org>

But what happens when you create another partition after you change the
"loggedness" of the partitioned table?

The new partition will have a property specified when the user creates it. That is, while the storage property of each storage unit (=partition) is basically independent, ALTER TABLE on a partitioned table is a convenient way to set the same property value to all its underlying storage units.

Well, my main take from that is we should strive to change all
subcommands together, in some principled way, rather than change only
one or some, in arbitrary ways.

I got an impression from the discussion that some form of consensus on the principle was made and you were trying to create a fix for REPLICA IDENTITY. Do you think the principle was unclear and we should state it first (partly to refresh people's memories)?

I'm kind of for it, but I'm hesitant to create the fix for all ALTER actions, because it may take a lot of time and energy as you were afraid. Can we define the principle, and then create individual fixes independently based on that principle?

Regards
Takayuki Tsunakawa

On 2020-Dec-09, tsunakawa.takay@fujitsu.com wrote:

From: Alvaro Herrera <alvherre@alvh.no-ip.org>

But what happens when you create another partition after you change the
"loggedness" of the partitioned table?

The new partition will have a property specified when the user creates
it. That is, while the storage property of each storage unit
(=partition) is basically independent, ALTER TABLE on a partitioned
table is a convenient way to set the same property value to all its
underlying storage units.

Well, that definition seems unfriendly to me. I prefer the stance that
if you change the value for the parent, then future partitions inherit
that value.

I got an impression from the discussion that some form of consensus on
the principle was made and you were trying to create a fix for REPLICA
IDENTITY. Do you think the principle was unclear and we should state
it first (partly to refresh people's memories)?

I think the principle was sound -- namely, that we should make all those
commands recurse by default, and for cases where it matters, the
parent's setting should determine the default for future children.

I'm kind of for it, but I'm hesitant to create the fix for all ALTER
actions, because it may take a lot of time and energy as you were
afraid. Can we define the principle, and then create individual fixes
independently based on that principle?

That seems acceptable to me, as long as we get all changes in the same
release. What we don't want (according to my reading of that
discussion) is to change the semantics of a few subcommands in one
release, and the semantics of a few other subcommands in another
release.

On Wed, Dec 9, 2020 at 6:22 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

On 2020-Dec-09, tsunakawa.takay@fujitsu.com wrote:

From: Alvaro Herrera <alvherre@alvh.no-ip.org>

But what happens when you create another partition after you change the
"loggedness" of the partitioned table?

The new partition will have a property specified when the user creates
it. That is, while the storage property of each storage unit
(=partition) is basically independent, ALTER TABLE on a partitioned
table is a convenient way to set the same property value to all its
underlying storage units.

Well, that definition seems unfriendly to me. I prefer the stance that
if you change the value for the parent, then future partitions inherit
that value.

I got an impression from the discussion that some form of consensus on
the principle was made and you were trying to create a fix for REPLICA
IDENTITY. Do you think the principle was unclear and we should state
it first (partly to refresh people's memories)?

I think the principle was sound -- namely, that we should make all those
commands recurse by default, and for cases where it matters, the
parent's setting should determine the default for future children.

I'm kind of for it, but I'm hesitant to create the fix for all ALTER
actions, because it may take a lot of time and energy as you were
afraid. Can we define the principle, and then create individual fixes
independently based on that principle?

That seems acceptable to me, as long as we get all changes in the same
release. What we don't want (according to my reading of that
discussion) is to change the semantics of a few subcommands in one
release, and the semantics of a few other subcommands in another
release.

I'm not sure how many more of such commands exist which require changes.

How about doing it this way?

1) Have a separate common thread listing the commands and specifying
clearly the agreed behaviour for such commands.
2) Whenever the separate patches are submitted(hopefully) by the
hackers for such commands, after the review we can make a note of that
patch in the common thread.
3) Once the patches for all the listed commands are
received(hopefully) , then they can be committed together in one
release.

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

On 2020-Dec-09, Bharath Rupireddy wrote:

I'm not sure how many more of such commands exist which require changes.

The other thread has a list. I think it is complete, but if you want to
double-check, that would be helpful.

How about doing it this way?

1) Have a separate common thread listing the commands and specifying
clearly the agreed behaviour for such commands.
2) Whenever the separate patches are submitted(hopefully) by the
hackers for such commands, after the review we can make a note of that
patch in the common thread.
3) Once the patches for all the listed commands are
received(hopefully) , then they can be committed together in one
release.

Sounds good. I think this thread is a good place to collect those
patches, but if you would prefer to have a new thread, feel free to
start one (I'd suggest CC'ing me and Tsunakawa-san).

From: Alvaro Herrera <alvherre@alvh.no-ip.org>

Well, that definition seems unfriendly to me. I prefer the stance that
if you change the value for the parent, then future partitions inherit
that value.

That would be right when the storage property is an optional specification such as fillfactor. For example, when I run ALTER TABLE mytable SET (fillfactor = 70) and then CREATE TABLE mytable_p1 PARTITION OF mytable, I find it nice that the fillfactor os mytable_p1 is also 70 (but I won't complain if it isn't, since I can run ALTER TABLE SET on the parent table again.)

OTOH, CREATE TABLE and CREATE UNLOGGED TABLE is an explicit request to create a logged and unlogged relation respectively. I feel it a strange? if CREATE TABLE mytable_p1 PARTITION OF mytable creates an unlogged partition.

I got an impression from the discussion that some form of consensus on
the principle was made and you were trying to create a fix for REPLICA
IDENTITY. Do you think the principle was unclear and we should state
it first (partly to refresh people's memories)?

I think the principle was sound -- namely, that we should make all those
commands recurse by default, and for cases where it matters, the
parent's setting should determine the default for future children.

Yeah, recurse by default sounded nice. But I didn't find a consensus related to "parent's setting should determine the default for future children." Could you point me there?

I'm kind of for it, but I'm hesitant to create the fix for all ALTER
actions, because it may take a lot of time and energy as you were
afraid. Can we define the principle, and then create individual fixes
independently based on that principle?

That seems acceptable to me, as long as we get all changes in the same
release. What we don't want (according to my reading of that
discussion) is to change the semantics of a few subcommands in one
release, and the semantics of a few other subcommands in another
release.

All fixes at one release seems constricting to me... Reading from the following quote in the past discussion, I understood consistency is a must and simultaneous release is an ideal. So, I think we can release individual fixes separately. I don't think it won't worsen the situation for users at least.

"try to make them all work the same, ideally in one release, rather than changing them at different times, back-patching sometimes, and having no consistency in the details.

BTW, do you think you can continue to work on your REPLICA IDENTITY patch soon?

Regards
Takayuki Tsunakawa

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>

I'm not sure how many more of such commands exist which require changes.

How about doing it this way?

1) Have a separate common thread listing the commands and specifying
clearly the agreed behaviour for such commands.
2) Whenever the separate patches are submitted(hopefully) by the
hackers for such commands, after the review we can make a note of that
patch in the common thread.
3) Once the patches for all the listed commands are
received(hopefully) , then they can be committed together in one
release.

That sounds interesting and nice. Having said that, I rather think it's better to release the fixes separately. Of course, we confirm the principle of consistency that individual fixes base on beforehand and what actions need sixing (SET LOGGED/UNLOGGED was missing in the past thread.)

Regards
Takayuki Tsunakawa

On Wed, Dec 09, 2020 at 09:52:17AM -0300, Alvaro Herrera wrote:

On 2020-Dec-09, tsunakawa.takay@fujitsu.com wrote:

The new partition will have a property specified when the user creates
it. That is, while the storage property of each storage unit
(=partition) is basically independent, ALTER TABLE on a partitioned
table is a convenient way to set the same property value to all its
underlying storage units.

Well, that definition seems unfriendly to me. I prefer the stance that
if you change the value for the parent, then future partitions inherit
that value.

That's indeed more interesting from the user perspective. So +1 from
me.
--
Michael

On Wed, Dec 09, 2020 at 10:56:45AM -0300, Alvaro Herrera wrote:

Sounds good. I think this thread is a good place to collect those
patches, but if you would prefer to have a new thread, feel free to
start one (I'd suggest CC'ing me and Tsunakawa-san).

There is an entry listed in the CF for this thread:
https://commitfest.postgresql.org/31/2857/

And it seems to me that waiting on author is most appropriate here, so
I have changed the entry to reflect that.
--
Michael