Re: [PATCH] Refactor bytea_sortsupport()

On Wed, Oct 09, 2024 at 05:39:22PM +0300, Aleksander Alekseev wrote:
> Attached is a PoC patch that fixes this. There are some TODOs and
> FIXMEs but all in all it works and passes the tests.

The patch has exactly three TODOs, and it is kind of hard to figure
out what your goal here is by only looking at the patch.

> The code becomes longer but the new code is simple and it's easier to
> understand. If we agree on this refactoring we could decompose
> adt/varlena.c into varlena.c and bytea.c - also something proposed by
> Tom. IMO it would be a good move but this is not implemented in the
> patch.
>
> Thoughts?

The point is that moving all the logic related to bytea is going to
reduce the risk of potential bugs if the varstring paths are
manipulated so as they accidentally impact the former, even if it
comes at a cost of duplicating some of it.

It seems to me that the patch should be more ambitious than just
trying to move the sort support functions and structures. How about
moving anything related to bytea to a new bytea.c, including the
in/out and receive/send functions. An idea would be to split the
patch into roughly two pieces, to prove the benefits that this can
have in the long-term:
1) Extract all the bytea-related code from varlena.c into its own
file, even if it comes copy-paste some of the structures and routines
shared by bytea and varstring. You would get the file structure done
first.
2) Maximize the simplifications in the new bytea.c and the former
varlena.c. This would show how this makes the code better for one,
for the other, or for both.
--
Michael