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
