Re: Add uuid_to_base32hex() and base32hex_to_uuid() built-in functions

Hi,

> I only rebased v3 and improved the commit messages, but I didn't
> account for Masahiko Sawada's feedback for 0002. Andrey, are you still
> working on this or others can pick it up?
>
> The patch is not on the commitfest, so I'm about to add it.

Here is patch v5 where I accounted for the previous feedback from
Masahiko Sawada and also made some other changes, see below.

> How about the error message like "invalid input length for type uuid"?
> I think "uuid" should be lower case as it indicates PostgreSQL uuid
> data type, and it's better to use %s format instead of directly
> writing "uuid" (see string_to_uuid() for example).

Makes sense. Fixed.

> As for the errdetail message, should we add "bytea" also after "got %d"?

You probably meant "got %d bytes", not "got %d bytea". I believe the
current message is fine, but maybe native speakers will correct us.

> We already have tests for casting bytes to integer data types in
> strings.sql. I suggest moving the casting tests from bytea to uuid
> into therel.

I disagree on the grounds that there are zero tests related to UUID in
strings.sql; uuid.sql is a more appropriate place for these tests IMO.
However if someone seconds the idea we can easily move the tests at
any time.

> For the uuid.sql file, we could add a test to verify that
> a UUID value remains unchanged when it's cast to bytea and back to
> UUID. For example,
>
> SELECT v = v::bytea::uuid as matched FROM gen_random_uuid() v;

Good point. Added.

> base32hex_encode() doesn't seem to add '=' paddings, but is it
> intentional? I don't see any description in RFC 4648 that we can omit
> '=' paddings.

You are right, both base32 and base32hex should add paddings;
substring() can be used if necessary. Fixed.

> I think the patch should add tests not only for uuid data type but
> also for general cases like other encodings.

Yes, and the good place for these tests would be closer to other tests
for encode() and decode() i.e. strings.sql. Fixed.

While working on it I noticed some inconsistencies between base32hex
implementation and our current implementation of base64. As an
example, we don't allow `=` input:

```
=# SELECT decode('=', 'base64');
ERROR: unexpected "=" while decoding base64 sequence
```

... while base32hex did. I fixed such inconsistencies too.

> In uuid.sql tests, how about adding some tests to check if base32hex
> maintains the sortability of UUIDv7 data?

Agree. Added.

> I think we should update the documentation in the uuid section about
> casting data between bytea and uuid. For references, we have a similar
> description for bytea and integer[1].

Fair point. Fixed.

--
Best regards,
Aleksander Alekseev