Hi Michael, > 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.
Many thanks for your feedback. Unfortunately I had no opportunity to work on this during the November commitfest, but I didn't forget about the patch and am going to submit an updated version, probably somewhere in January. -- Best regards, Aleksander Alekseev
