Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions
| От | Chao Li |
|---|---|
| Тема | Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions |
| Дата | |
| Msg-id | 52CDB681-F0AD-4024-9BF4-5FB4F137C9A2@gmail.com обсуждение исходный текст |
| Ответ на | [PATCH] Add hints for invalid binary encoding names in encode/decode functions (Sugamoto Shinya <shinya34892@gmail.com>) |
| Ответы |
Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions
Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions |
| Список | pgsql-hackers |
> On Nov 8, 2025, at 14:25, 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`.
>
> 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
aencoding 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/
В списке pgsql-hackers по дате отправления: