On Mon, 18 Nov 2024 at 08:57, Zhijie Hou (Fujitsu)
<houzj.f...@fujitsu.com> wrote:
>
> On Saturday, November 16, 2024 2:41 AM Shlok Kyal <shlok.kyal....@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
