From: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: [PATCH] Refactor bytea_sortsupport() |
Date: | 2024-12-05 08:11:55 |
Message-ID: | CAJ7c6TN2O1eWjspccEuTeWEfO4YCRHo6VqcOWJWSu+SDZvPc-w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Daniil Davydov | 2024-12-05 08:13:08 | [BUG] Assert always fails while checking whether local buffer is dirty of is exclusively locked |
Previous Message | Aleksander Alekseev | 2024-12-05 08:08:42 | Re: [PATCH] Refactor SLRU to always use long file names |