Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

Thanks for providing the comments.

On Sat, 16 Nov 2024 at 17:29, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Sat, 16 Nov 2024 at 00:10, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)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