On Mon, Jan 09, 2023 at 09:36:12AM +0100, Peter Eisentraut wrote: > On 04.01.23 01:37, Nathan Bossart wrote: >> On Tue, Dec 20, 2022 at 06:30:40AM +0100, Peter Eisentraut wrote: >> > + * A value may be NULL. If the corresponding placeholder is found in the >> > + * input string, the whole function returns NULL. >> >> This appears to be carried over from BuildRestoreCommand(), and AFAICT it >> is only necessary because pg_rewind doesn't support %r in restore_command. >> IMHO this behavior is counterintuitive and possibly error-prone and should >> result in an ERROR instead. Since pg_rewind is the only special case, it >> could independently check for %r before building the command. > > Yeah, this annoyed me, too. I have now changed it so that a NULL "value" is > the same as an unsupported placeholder. This preserves the existing > behavior. > > (Having pg_rewind check for %r itself would probably require replicating > most of the string processing logic (consider something like "%%r"), so it > didn't seem appealing.)
Sounds good to me. > + nativePath = pstrdup(path); > + make_native_path(nativePath); > + nativePath = pstrdup(xlogpath); > + make_native_path(nativePath); Should these be freed? -- Nathan Bossart Amazon Web Services: https://aws.amazon.com