Re: Proposal: Filter irrelevant change before reassemble transactions during logical decoding

On Thu, 20 Mar 2025 at 18:09, Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
> On Thu, Mar 13, 2025 at 5:49 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> >
> > Moving this patch to the next CF as this patch needs more design level
> > inputs which may not be feasible in this CF but do continue to review
> > the patch.
> >
> > regards,
> > Ajin Cherian
> > Fujitsu Australia
>
> Rebased the patch as it no longer applied.
>

Hi Ajin,

I reviewed v16-0002 patch, here are my comments:

1. Guc CHANGES_THRESHOLD_FOR_FILTER was introduced in 0001 patch,
should this change be in 0001 patch?

- * temporarily suspended. Filtering resumes after processing every 100 changes.
+ * temporarily suspended. Filtering resumes after processing
CHANGES_THRESHOLD_FOR_FILTER
+ * changes.

2. Following comment is repeated inside DecodeInsert, DecodeUpdate,
DecodeDelete, DecodeMultiInsert, DecodeSpecConfirm
+ /*
+ * When filtering changes, determine if the relation associated with the change
+ * can be skipped. This could be because the relation is unlogged or because
+ * the plugin has opted to exclude this relation from decoding.
+ */
+ if (SkipThisChange(ctx, buf->origptr, XLogRecGetXid(r), &target_locator,

Since this comment is the same, should we remove it from the above
functions and add this where function 'SkipThisChange' is defined?

Thanks and Regards,
Shlok Kyal