On Wed, Feb 01, 2023 at 09:34:44PM -0500, Tom Lane wrote: > Michael Paquier <mich...@paquier.xyz> writes: >> Hmm. Isn't that something that we should also document in startup.c >> where both routines are defined? If we begin to use >> PreRestoreCommand() and PostRestoreCommand() in more code paths in the >> future, that could be again an issue. > > I was vaguely wondering about removing both of those functions > in favor of an integrated function that does a system() call > with those things before and after it.
It seems to me that this is pretty much the same as storing in_restore_command in shell_restore.c, and that for recovery modules this comes down to the addition of an extra callback called in startup.c to check if the flag is up or not. Now the patch is doing things the opposite way: like on HEAD, store the flag in startup.c but switch it at will with the routines in startup.c. I find the approach of the patch a bit more intuitive, TBH, as that makes the interface simpler for other recovery modules that may want to switch the flag back-and-forth, and I suspect that there may be cases in recovery modules where we'd still want to switch the flag, but not necessarily link it to system(). -- Michael
signature.asc
Description: PGP signature
