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

On Mon, 18 Nov 2024 at 08:57, Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Saturday, November 16, 2024 2:41 AM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> >
> > >
> > Thanks for providing the comments. I have fixed all the comments and attached
> > the updated patch.
>
> Thanks for the patch. I have one comment for the following codes:
>
> +               /*
> +                * Bitmap of published columns when publish_generated_columns is
> +                * 'false' and no column list is specified.
> +                */
> +               columns = pub_form_cols_map(relation, false);
> +
> +               /*
> +                * Attnums in the bitmap returned by RelationGetIndexAttrBitmap are
> +                * offset (to handle system columns the usual way), while column list
> +                * does not use offset, so we can't do bms_is_subset(). Instead, we
> +                * have to loop over the idattrs and check all of them are in the
> +                * list.
> +                */
> +               x = -1;
> +               while ((x = bms_next_member(idattrs, x)) >= 0)
> +               {
> ...
> +               }
>
>
> It doesn't seem necessary to build a bitmap and then iterator the replica
> identity bitmap. Instead, we can efficiently traverse the columns as follows:
>
> for (int i = 0; i < desc->natts; i++)
> {
>         Form_pg_attribute att = TupleDescAttr(desc, i);
>
>         if (!att->attisdropped && att->attgenerated &&
>                 bms_is_member(att->attnum - FirstLowInvalidHeapAttributeNumber,
>         idattrs))
>         {
>                 result = true;
>                 break;
>         }
> }
>
> Best Regards,
> Hou zj

Thanks for providing the comments.
I agree with your approach and updated the same in the v7 patch [1].

[1]: https://www.postgresql.org/message-id/CANhcyEUi6T%2B0O83LEsG6jOJFL3BY_WD%3DvZ73bt0FRUcJHRt%3DsQ%40mail.gmail.com

Thanks and Regards,
Shlok Kyal