From: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | RE: Adding a LogicalRepWorker type field |
Date: | 2023-08-18 08:16:41 |
Message-ID: | OS0PR01MB571661AA5C700358460863C6941BA@OS0PR01MB5716.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Best Regards,
Hou zj
From | Date | Subject | |
---|---|---|---|
Next Message | Christoph Berg | 2023-08-18 08:22:47 | Re: Remove distprep |
Previous Message | Bagga, Rishu | 2023-08-18 08:12:41 | Re: SLRUs in the main buffer pool - Page Header definitions |