Re: Adding a LogicalRepWorker type field

On Fri, Aug 18, 2023 at 8:50 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Aug 14, 2023 at 12:08 PM Peter Smith <smithpb2250@gmail.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?
>

I feel it does give a tad bit more clarity for cases where we have
'else' part with no clear comments or relevant keywords. As an
example, in function 'should_apply_changes_for_rel' , we have:
        else
                return (rel->state == SUBREL_STATE_READY ||
                                (rel->state == SUBREL_STATE_SYNCDONE &&
                                 rel->statelsn <= remote_final_lsn));

It is difficult to figure out which worker is this if I do not know
the concept completely;   'case WORKERTYPE_APPLY' makes it better for
the reader to understand.

thanks
Shveta