Re: Adding a LogicalRepWorker type field

On Fri, Aug 18, 2023 at 6:16 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Friday, August 18, 2023 11:20 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Aug 14, 2023 at 12:08 PM Peter Smith <smithpb2250(at)gmail(dot)com>
> > wrote:
> > >
> > > The main patch for adding the worker type enum has been pushed [1].
> > >
> > > Here is the remaining (rebased) patch for changing some previous
> > > cascading if/else to switch on the LogicalRepWorkerType enum instead.
> > >
> >
> > I see this as being useful if we plan to add more worker types. Does anyone else
> > see this remaining patch as an improvement?
>
> +1
>
> I have one comment for the new error message.
>
> + case WORKERTYPE_UNKNOWN:
> + ereport(ERROR, errmsg_internal("should_apply_changes_for_rel: Unknown worker type"));
>
> I think reporting an ERROR in this case is fine. However, I would suggest
> refraining from mentioning the function name in the error message, as
> recommended in the error style document [1]. Also, it appears we could use
> elog() here.
>
> [1] https://www.postgresql.org/docs/devel/error-style-guide.html

OK. Modified as suggested. Anyway, getting these errors should not
even be possible, so I added a comment to emphasise that.

PSA v9

------
Kind Regards,
Peter Smith.
Fujitsu Australia