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

On Fri, 29 Nov 2024 at 13:38, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
>
> On Thu, 28 Nov 2024 at 16:38, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Nov 21, 2024 at 5:30 PM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
> > >
> >
> > Review comments:
> > ===============
> > 1.
> > +
> > + /*
> > + * true if all generated columns which are part of replica identity are
> > + * published or the publication actions do not include UPDATE or DELETE.
> > + */
> > + bool replident_valid_for_update;
> > + bool replident_valid_for_delete;
> >
> > These are too generic names for the purpose they are used. How about
> > instead name them as gencols_valid_for_update and
> > gencols_valid_for_delete?
> >
> > 2. The comments atop RelationBuildPublicationDesc() is only about row
> > filter. We should update it for column list and generated columns as
> > well.
> >
> > 3. It is better to merge the functionality of the invalid column list
> > and unpublished generated columns as proposed by Hou-San above.
> >
>
> Thanks for reviewing the patch. I have addressed the comments and
> updated the patch.

Shouldn't unpublished_gen_col be set only if the column list is absent?
- /* Transform the column list datum to a bitmapset. */
- columns = pub_collist_to_bitmapset(NULL, datum, NULL);
+ /* Check if generated column is part of REPLICA IDENTITY */
+ *unpublished_gen_col |= att->attgenerated;

- /* Remember columns that are part of the REPLICA IDENTITY */
- idattrs = RelationGetIndexAttrBitmap(relation,
-
INDEX_ATTR_BITMAP_IDENTITY_KEY);
+ if (columns == NULL)
+ {
+ /* Break the loop if unpublished generated
columns exist. */
+ if (*unpublished_gen_col)
+ break;
+
+ /* Skip validating the column list since it is
not defined */
+ continue;
+ }

This scenario worked in v11 but fails in v12:
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(b);
UPDATE testpub_gencol SET a = 100 WHERE a = 1;

Regards,
Vignesh