Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions
Date
Msg-id CAHGQGwE_bCcf3uZ4wWF52h1TAbeQ3hPmn-cr7HAvEppiVXEyVA@mail.gmail.com
Whole thread Raw
In response to [PATCH] Add hints for invalid binary encoding names in encode/decode functions  (Sugamoto Shinya <shinya34892@gmail.com>)
List 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 by date:

Previous
From: Jakub Wartak
Date:
Subject: Re: contrib/pg_stat_tcpinfo
Next
From: Chao Li
Date:
Subject: gen_guc_tables.pl: Validate required GUC fields before code generation