On Wed, Dec 2, 2020 at 3:57 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Wed, Dec 2, 2020 at 1:52 PM tsunakawa.ta...@fujitsu.com > <tsunakawa.ta...@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] in my previous mail. [1] 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