Re: Skipping logical replication transactions on subscriber side

On Wed, Nov 17, 2021 at 7:46 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Tue, Nov 16, 2021 at 12:01 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Nov 15, 2021 at 11:43 PM vignesh C <vignesh21@gmail.com> wrote:
> > >
> > > On Mon, Nov 15, 2021 at 2:48 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Mon, Nov 15, 2021 at 4:49 PM Greg Nancarrow <gregn4422@gmail.com> wrote:
> > > > >
> > > > > On Mon, Nov 15, 2021 at 1:49 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > > > >
> >
> > Right. I've fixed this issue and attached an updated patch.
>
> Few comments:

Thank you for the comments!

> 1) should we set subwentry to NULL to handle !create && !found case
> or we could return NULL similar to the earlier function.
> +static PgStat_StatSubWorkerEntry *
> +pgstat_get_subworker_entry(PgStat_StatDBEntry *dbentry, Oid subid,
> Oid subrelid,
> +                                                  bool create)
> +{
> +       PgStat_StatSubWorkerEntry *subwentry;
> +       PgStat_StatSubWorkerKey key;
> +       bool            found;
> +       HASHACTION      action = (create ? HASH_ENTER : HASH_FIND);
> +
> +       key.subid = subid;
> +       key.subrelid = subrelid;
> +       subwentry = (PgStat_StatSubWorkerEntry *)
> hash_search(dbentry->subworkers,
> +
>                                            (void *) &key,
> +
>                                            action, &found);
> +
> +       /* If not found, initialize the new one */
> +       if (create && !found)

It's better to return NULL if !create && !found. WIll fix.

>
> 2) Should we keep the line width to 80 chars:
> +/* ----------
> + * PgStat_MsgSubWorkerError            Sent by the apply worker or
> the table sync worker to
> + *                                                             report
> the error occurred during logical replication.
> + * ----------
> + */
> +#define PGSTAT_SUBWORKERERROR_MSGLEN 256
> +typedef struct PgStat_MsgSubWorkerError
> +{

Hmm, pg_indent seems not to fix it. Anyway, will fix.

I'll fix an updated patch.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/