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. 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. Regards, Vignesh