> On Oct 10, 2025, at 10:54, jian he <[email protected]> wrote: > >> 5 >> ``` >> +static void >> +CopyRelTo(CopyToState cstate, Relation rel, Relation root_rel, >> + uint64 *processed) >> ``` >> >> Instead of using a pointer to pass out processed count, I think it’s better >> to return the process count. I understand the current implementation allows >> continuous increment while calling this function in a loop. However, it’s a >> bit error-prone, a caller must make sure “processed” is well initialized. >> With returning a unit64, the caller’s code is still simple: >> >> ``` >> processed += CopyRelTo(cstate, …); >> ``` >> >> pgstat_progress_update_param was within CopyRelTo. >> so we have to pass (uint64 *processed) to CopyRelTo. >> Am I missing something? >> >> >> Make sense. I didn’t notice postage_progress_update_param. So, “processed” >> is both input and output. In that case, I think the comment for parameter >> “processed” should be enhanced, for example: >> > if your function is: > > static processed CopyRelationTo(CopyToState cstate, Relation rel, > Relation root_rel, uint64 *processed); > > where function return value is also passed as function argument, > I think it will lead to more confusion.
I am not suggesting add a return value to the function. My comment was just to enhance the parameter comment of “processed”. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
