On Thu, 21 Nov 2024 at 15:26, vignesh C <vignes...@gmail.com> wrote: > > On Tue, 19 Nov 2024 at 19:12, Shlok Kyal <shlok.kyal....@gmail.com> wrote: > > > > On Tue, 19 Nov 2024 at 14:39, Zhijie Hou (Fujitsu) > > <houzj.f...@fujitsu.com> wrote: > > > > > > On Tuesday, November 19, 2024 3:15 PM Shlok Kyal > > > <shlok.kyal....@gmail.com> wrote: > > > > > > > > > > > I noticed that we can add 'publish_generated_columns = true' for the > > > > case of > > > > generated column. So we won't need to remove the test. I have made the > > > > changes in v9 patch [1]. > > > > > > I think this would unexpectedly change the original purpose of that > > > testcase, > > > which is to test the bug mentioned in commit b797def. > > > > > > Basically, I expected the new testcase to fail if we remove the codes fix > > > added in > > > b797def, but the new testcase can pass even after that. > > > > > > If we confirmed that that bug will never be triggered after applying the > > > fix in > > > the thread, it would be better Tt remove that testcase and mention it in > > > the > > > commit message. > > > > > I agree that we can remove the test. I debugged and found the test > > modified in above patch does not hit the condition added in commit > > adedf54. > > Also, according to me we cannot trigger the bug after the fix in this > > thread. So, I think we can remove the testcase. > > > > I have attached the latest patch with an updated commit message and > > also removed the testcase. > > Few comments: > 1) This seems like a copy paste from > pub_collist_contains_invalid_column, the comments should be updated > according to this function: > + /* > + * For a partition, if pubviaroot is true, find the topmost > ancestor that > + * is published via this publication as we need to use its > column list for > + * the changes. > + * > + * Note that even though the column list used is for an ancestor, the > + * REPLICA IDENTITY used will be for the actual child table. > + */ > + if (pubviaroot && relation->rd_rel->relispartition) > > 2) Here drop index is not required as the drop table will take care of > dropping the index too: > +UPDATE testpub_gencol SET a = 100 WHERE a = 1; > +DROP PUBLICATION pub_gencol; > +DROP INDEX testpub_gencol_idx; > +DROP TABLE testpub_gencol; > +RESET client_min_messages; > Thanks for the comments. I have fixed the comments and attached the updated patch.
Thanks and Regards, Shlok Kyal
v11-0001-Disallow-UPDATE-DELETE-on-table-with-generated-c.patch
Description: Binary data
