Re: Logical Replication of sequences

On Mon, 22 Sept 2025 at 12:03, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Sep 18, 2025 at 4:07 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > Thanks for the comments, these are handled in the attached patch.
> >
>
> Please find a few comments:
>
>
> patch005:
> 1)
> GetSubscriptionRelations:
> + /* Skip sequences if they were not requested */
> + if (!get_sequences && (relkind == RELKIND_SEQUENCE))
> + continue;
> +
> + /* Skip tables if they were not requested */
> + if (!get_tables && (relkind != RELKIND_SEQUENCE))
> + continue;
>
> The use of negated conditions makes the logic harder to follow,
> especially in the second if block.
>
> Can we write it as:
> bool is_sequence = (relkind == RELKIND_SEQUENCE);
>
> /* Skip if the relation type is not requested */
> if ((get_tables && is_sequence) ||
> (get_sequences && !is_sequence))
> continue;
>
> Or at-least:
> /* Skip sequences if they were not requested */
> if (get_tables && (relkind == RELKIND_SEQUENCE))
> continue;
>
> /* Skip tables if they were not requested */
> if (get_sequences && (relkind != RELKIND_SEQUENCE))
> continue;

I felt this would not work. Say we want both sequences and tables,
won't it skip the sequence this way from:
if (get_tables && (relkind == RELKIND_SEQUENCE))
continue;

Rest of the comments were fixed, also the comment from [1] is fixed in
the attached patch.
[1] - https://www.postgresql.org/message-id/CAJpy0uAdD9XtZCE34BJhbvncMgfmMuTS0ZXLP1P=g+wpRC8vqQ@mail.gmail.com

Regards,
Vignesh