Re: Common function for percent placeholder replacement

Nathan Bossart <nathandbossart@gmail.com>

From: Nathan Bossart <nathandbossart@gmail.com>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org
Date: 2023-01-04T00:37:00Z
Lists: pgsql-hackers
In general, +1.

On Tue, Dec 20, 2022 at 06:30:40AM +0100, Peter Eisentraut wrote:
> (Still need to think about Robert's comment about lack of error context.)

Would adding the name of the GUC be sufficient?

	ereport(ERROR,
			(errmsg("could not build %s", guc_name),
			 errdetail("string ends unexpectedly after escape character \"%%\"")));

> + * 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.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com



Commits

  1. Code cleanup

  2. Common function for percent placeholder replacement