Thanks for providing the comments. On Sat, 16 Nov 2024 at 17:29, vignesh C <vignes...@gmail.com> wrote: > > On Sat, 16 Nov 2024 at 00:10, Shlok Kyal <shlok.kyal....@gmail.com> wrote: > > > > Thanks for providing the comments. I have fixed all the comments and > > attached the updated patch. > > Few comments: > 1) The replident_has_valid_gen_cols flag is set when either an update > or delete operation is published by the publication. > + /* > + * Check if all columns which are part of the REPLICA > IDENTITY is > + * published. > + */ > + if (!pubform->pubgencols && > + (pubform->pubupdate || pubform->pubdelete) && > + replident_has_unpublished_gen_col(pubid, > relation, ancestors, > + > pubform->pubviaroot)) > + pubdesc->replident_has_valid_gen_cols = false; > > You should create two separate flags—one for updates and one for > deletes—and set them accordingly, based on the operation being > published. This is similar to how the cols_valid_for_update and > cols_valid_for_delete flags are handled for column lists. > > As shown in the test below, the delete operation fails even though the > delete operation is not published by the pub_gencol publication: > CREATE TABLE testpub_gencol (a INT, b INT GENERATED ALWAYS AS (a + 1) > STORED NOT NULL); > CREATE UNIQUE INDEX testpub_gencol_idx ON testpub_gencol (b); > ALTER TABLE testpub_gencol REPLICA IDENTITY USING index testpub_gencol_idx; > > CREATE PUBLICATION pub_gencol FOR TABLE testpub_gencol with ( PUBLISH > = 'update'); > > -- This should be successful, since the publication is not publishing delete: > postgres=# delete from testpub_gencol ; > ERROR: cannot delete from table "testpub_gencol" > DETAIL: Replica identity consists of an unpublished generated column. > I have fixed the issue.
> 2) Since the code in replident_has_unpublished_gen_col and > pub_collist_contains_invalid_column is largely identical, we can > consolidate them into a single function that handles both column lists > and relation columns. The function name, header comments, and internal > comments should be updated accordingly. I tried to merge both functions but it required extra checks. I think it would make the patch a little complicated to review. I have attached the updated version of the patch. Thanks and Regards, Shlok Kyal
v7-0001-Disallow-UPDATE-DELETE-on-table-with-generated-co.patch
Description: Binary data
