On 2025-Dec-29, Álvaro Herrera wrote: > On 2025-Dec-24, Ashutosh Bapat wrote:
> > It's also quite easy to confuse between these functions and > > replorigin_session_reset(). It's not clear where the boundaries of the > > latter end and where those of the new ones start. I think the latter > > deals with the shared memory structures while the new ones deal with > > the backend local state. And then there's replorigin_reset() which > > adds to the confusion. That function doesn't call > > replorigin_session_reset() which the other two callers of > > replorigin_session_clear_state() call. Why? I think there is more to > > clean here than what's in the patch. That doesn't mean that we cannot > > accept this patch without larger cleanup, but it should not add to the > > existing confusion. I think we should just rename the worker.c function to have a less generic-sounding name (so that it becomes more obvious that it's a worker.c-specific function); have both replorigin_session_clear_state() and replorigin_xact_clear_state() be a single function, with a boolean flag to indicate whether to reset replorigin_session_origin or not; and have replorigin_session_reset() call replorigin_session_clear_state() internally rather than require every single of its callers do it separately, which IMO makes no sense. With these three changes I think the code would become quite clear. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ Thou shalt check the array bounds of all strings (indeed, all arrays), for surely where thou typest "foo" someone someday shall type "supercalifragilisticexpialidocious" (5th Commandment for C programmers)
