Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions

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

In response to

Browse pgsql-hackers by date

  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