On Fri, Dec 4, 2020 at 8:22 AM tsunakawa.ta...@fujitsu.com <tsunakawa.ta...@fujitsu.com> wrote: > > From: Bharath Rupireddy <bharath.rupireddyforpostg...@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