| From: | Chao Li <li(dot)evan(dot)chao(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 09:06:07 |
| Message-ID: | 52CDB681-F0AD-4024-9BF4-5FB4F137C9A2@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Nov 8, 2025, at 14:25, 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`.
>
> 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 briefly considered adding a small helper function to build the hint string,
> but decided against it since keeping the codebase simple felt preferable, and
> new binary encodings are not added very often.
>
> I’d be happy to hear any opinions or suggestions.
>
> Patch attached.
>
> Best regards,
> Shinya Sugamoto
> <0001-Added-error-hints-for-invalid-binary-encoding-names-.patch>
1.
```
- errmsg("unrecognized encoding: \"%s\"", namebuf)));
+ errmsg("unrecognized encoding: \"%s\"", namebuf),
+ errhint("Valid binary encodings are: %s",
+ "\"hex\", \"base64\", \"base64url\", \"escape\".")));
```
I think hardcoding the encoding list is fragile. AFAIK, “base64url” was newly added just a couple of months ago.
The list is defined in encode.c (search for enclist in the file), I guess we can add a function to return a string with a encoding names.
2
```
+ errhint("Valid binary encodings are: %s",
+ "\"hex\", \"base64\", \"base64url\", \"escape\".")));
```
Looks like putting punctuation inside %s is not normal. By looking at other hint messages, they usually do something like:
```
errhint("Perhaps you meant the option \"%s\".”, value)
```
But I think if you address comment 1, then this one will be addressed as well.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2025-11-10 09:11:40 | Re: Newly created replication slot may be invalidated by checkpoint |
| Previous Message | Shlok Kyal | 2025-11-10 09:03:54 | Re: Logical Replication of sequences |