| From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
|---|---|
| To: | Sugamoto Shinya <shinya34892(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions |
| Date: | 2025-11-10 08:39:31 |
| Message-ID: | CAHGQGwE_bCcf3uZ4wWF52h1TAbeQ3hPmn-cr7HAvEppiVXEyVA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, Nov 8, 2025 at 3:26 PM Sugamoto Shinya <shinya34892(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2025-11-10 08:42:00 | gen_guc_tables.pl: Validate required GUC fields before code generation |
| Previous Message | Jakub Wartak | 2025-11-10 08:34:58 | Re: contrib/pg_stat_tcpinfo |