Re: [PATCH] Refactor bytea_sortsupport(), take two

On Mon, Sep 15, 2025 at 3:40 PM Aleksander Alekseev
<aleksander@tigerdata.com> wrote:
>
> > Don't these types disallow NUL bytes regardless of locale / character set?
>
> You are right, they do. Here is the patch v3 with corrected comments.

Hi Aleksander,

- * Relies on the assumption that text, VarChar, BpChar, and bytea all have the
- * same representation.  Callers that always use the C collation (e.g.
- * non-collatable type callers like bytea) may have NUL bytes in their strings;
- * this will not work with any other collation, though.
+ * Relies on the assumption that text, VarChar, and BpChar all have the
+ * same representation. These text types cannot contain NUL bytes.

AFAICS, the only reaon to mention NUL bytes here before was because of
bytea -- sirnce bytea is being removed from this path, I don't see a
need to mention mention NUL bytes here. It'd be relevant if any
simplification is possible in functions that previously may have had
to worry about NUL bytes. I don't immediately see any such
opportunities, though. Are there?

+ * Note: text types (text, varchar, bpchar) cannot contain NUL bytes,
+ * so we don't need to worry about NUL byte handling here.

Same here. Also, "Note" is stronger than a normal comment, maybe to
call attention to complex edge cases or weird behaviors.

+#if SIZEOF_DATUM == 8

We recently made all datums 8 bytes.

Some comments are randomly different than the equivalents in
varlena.c. It's probably better if same things remain the same, but
there's nothing wrong either.

"Lastly, the performance and memory consumption could be optimized a little for
the bytea case."

Is this a side effect of the patch, or a possibility for future work?
It's not clear.

The rest seems fine at a glance.
--
John Naylor
Amazon Web Services