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

From Chao Li
Subject Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions
Date
Msg-id 52CDB681-F0AD-4024-9BF4-5FB4F137C9A2@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>)
Responses 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
List 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 by date:

Previous
From: Shlok Kyal
Date:
Subject: Re: Logical Replication of sequences
Next
From: Amit Kapila
Date:
Subject: Re: Newly created replication slot may be invalidated by checkpoint