Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions
| От | Fujii Masao |
|---|---|
| Тема | Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions |
| Дата | |
| Msg-id | CAHGQGwE_bCcf3uZ4wWF52h1TAbeQ3hPmn-cr7HAvEppiVXEyVA@mail.gmail.com обсуждение исходный текст |
| Ответ на | [PATCH] Add hints for invalid binary encoding names in encode/decode functions (Sugamoto Shinya <shinya34892@gmail.com>) |
| Список | pgsql-hackers |
On Sat, Nov 8, 2025 at 3:26 PM Sugamoto Shinya <shinya34892@gmail.com> wrote:
>
> Hi,
>
> When users pass an invalid encoding name to the built-in functions
> `encode()` or `decode()`, PostgreSQL currently reports only:
>
> ERROR: unrecognized encoding: "xxx"
>
> This patch adds an error hint listing the valid encoding names,
> so users can immediately see the supported options.
>
> Example:
>
> SELECT encode('\x01'::bytea, 'invalid');
> ERROR: unrecognized encoding: "invalid"
> HINT: Valid binary encodings are: "hex", "base64", "base64url", "escape".
>
> This change applies to both `binary_encode()` and `binary_decode()` in `encode.c`,
> and adds regression tests under `src/test/regress/sql/strings.sql`.
+1
- errmsg("unrecognized encoding: \"%s\"", namebuf)));
+ errmsg("unrecognized encoding: \"%s\"", namebuf),
+ errhint("Valid binary encodings are: %s",
+ "\"hex\", \"base64\", \"base64url\", \"escape\".")));
Since neither encode.c nor the documentation for encode() use
the term "binary encoding", I think just "encodings" is sufficient
in the hint message.
Also, the colon after "encodings”"doesn't seem necessary.
+-- invalid encoding names should report the valid options
+SELECT encode('\x01'::bytea, 'invalid'); -- error
I'd like to improve the comment like:
"report an error with a hint listing valid encodings when an invalid
encoding is specified".
> Although this is a small change, let me share a few considerations behind it:
>
> - I extracted the valid encodings from the hint messages and used a format specifier like
> `Valid binary encodings are: %s`, so that we avoid scattering those fixed strings
> across translation files.
I understand your point. But since new encodings are rarely added as you told,
I'm fine to list them directly in the hint instead of using %s,
similar to other hints like:
src/backend/commands/dbcommands.c:
errhint("Valid strategies are \"wal_log\" and \"file_copy\".")));
Regards,
--
Fujii Masao
В списке pgsql-hackers по дате отправления: