From: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
---|---|
To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Common function for percent placeholder replacement |
Date: | 2023-01-09 08:36:12 |
Message-ID: | 1c2204c9-a38e-3075-56dc-eef9a103a645@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 04.01.23 01:37, Nathan Bossart wrote:
> 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 \"%%\"")));
done
The errors now basically look like an invalid GUC value.
>> + * 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.)
Attachment | Content-Type | Size |
---|---|---|
v4-0001-Common-function-for-percent-placeholder-replaceme.patch | text/plain | 17.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Aleksander Alekseev | 2023-01-09 08:41:19 | Re: RFC: logical publication via inheritance root? |
Previous Message | Ankit Kumar Pandey | 2023-01-09 08:34:08 | Re: Todo: Teach planner to evaluate multiple windows in the optimal order |