[pg_rewind] use the passing callback instead of global function

Started by Junwang Zhaoalmost 3 years ago3 messageshackers
Jump to latest
#1Junwang Zhao
zhjwpku@gmail.com

`local_traverse_files` and `libpq_traverse_files` both have a
callback parameter but instead use the global process_source_file
which is no good for function encapsulation.

--
Regards
Junwang Zhao

Attachments:

0001-pg_rewind-use-the-passing-callback-instead-of-global.patchapplication/octet-stream; name=0001-pg_rewind-use-the-passing-callback-instead-of-global.patchDownload+2-3
#2Richard Guo
guofenglinux@gmail.com
In reply to: Junwang Zhao (#1)
Re: [pg_rewind] use the passing callback instead of global function

On Wed, Apr 26, 2023 at 9:51 AM Junwang Zhao <zhjwpku@gmail.com> wrote:

`local_traverse_files` and `libpq_traverse_files` both have a
callback parameter but instead use the global process_source_file
which is no good for function encapsulation.

Nice catch. This should be a typo introduced by 37d2ff38.

While this patch is doing it correctly, I'm wondering that since both
kinds of source server (libpq and local) are using the same function
(i.e. process_source_file) to process source file list for
traverse_files operations, do we really need to provide a callback? Or
will there be some kind of source server that may use different source
file processing function?

Thanks
Richard

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Richard Guo (#2)
Re: [pg_rewind] use the passing callback instead of global function

On 26 Apr 2023, at 10:33, Richard Guo <guofenglinux@gmail.com> wrote:

On Wed, Apr 26, 2023 at 9:51 AM Junwang Zhao <zhjwpku@gmail.com <mailto:zhjwpku@gmail.com>> wrote:
`local_traverse_files` and `libpq_traverse_files` both have a
callback parameter but instead use the global process_source_file
which is no good for function encapsulation.

Nice catch. This should be a typo introduced by 37d2ff38.

Agreed, I'll look at applying this after some testing.

While this patch is doing it correctly, I'm wondering that since both
kinds of source server (libpq and local) are using the same function
(i.e. process_source_file) to process source file list for
traverse_files operations, do we really need to provide a callback? Or
will there be some kind of source server that may use different source
file processing function?

While there isn't one right now, removing the callback seems like imposing a
restriction that the refactoring in 37d2ff38 aimed to avoid.

--
Daniel Gustafsson