On Jul 11, 2025, at 04:26, Florents Tselai <florents.tselai@gmail.com> wrote:
> Attached
Thank you! This looks great. The attached revision makes a a couple of minor changes:
* Change the line wrap of the docs to be more like the rest of func.sgml
* Remove an unnecessary nested if statement
* Removed `==` from one of the test comments
* Ran pgindent to create the attached patch
A few other brief comments, entirely stylistic:
* I kind of expected pg_base64url_encode to appear immediate after pg_base64_encode. In other words, to see the two
usesof pg_base64_encode_internal adjacent to each other. I think that’s more typical of the project standard. Same for
thefunctions that call pg_base64_decode_internal.
* There are a few places where variable definition has been changed without changing the meaning, for example:
- const char *srcend = src + len,
- *s = src;
+ const char *srcend = src + len;
+ const char *s = src;
Even if this is desirable, it might make sense to defer pure formatting changes to a separate patch.
* You define return variables in functions like pg_base64url_enc_len rather than just returning the outcome of an
expression.The latter is what I see in pg_base64_enc_len, so I think would be more consistent. Io other words:
```patch
--- a/src/backend/utils/adt/encode.c
+++ b/src/backend/utils/adt/encode.c
@@ -470,8 +470,6 @@ pg_base64_dec_len(const char *src, size_t srclen)
static uint64
pg_base64url_enc_len(const char *src, size_t srclen)
{
- uint64 result;
-
/*
* Base64 encoding converts 3 bytes into 4 characters Formula: ceil(srclen
* / 3) * 4
@@ -479,10 +477,8 @@ pg_base64url_enc_len(const char *src, size_t srclen)
* Unlike standard base64, base64url doesn't use padding characters when
* the input length is not divisible by 3
*/
- result = (srclen + 2) / 3 * 4; /* ceiling division by 3, then multiply by
+ return (srclen + 2) / 3 * 4; /* ceiling division by 3, then multiply by
* 4 */
-
- return result;
}
static uint64
```
I suspect these are the sorts of things a committer would tweak/adjust before committing, just thinking about getting
aheadof that. I think it’s ready.
Best,
David
