Hi,
On 2023-02-08 11:16:47 +0100, Tomas Vondra wrote:
> On 2/7/23 21:18, Andres Freund wrote:
> >
> > Independent of this failure, I'm worried about the cost/benefit analysis of a
> > pglz change that changes this much at once. It's quite hard to review.
> >
>
> I agree.
>
> I think I managed to understand what the patch does during the review,
> but it's so much harder - it'd definitely be better to have this split
> into smaller parts, somehow. Interestingly enough the commit message
> actually says this:
>
> This patch accumulates several changes to pglz compression:
> 1. Convert macro-functions to regular functions for readability
> 2. Use more compact hash table with uint16 indexes instead of pointers
> 3. Avoid prev pointer in hash table
> 4. Use 4-byte comparisons during a search instead of 1-byte
> comparisons
>
> Which I think is a pretty good recipe how to split the patch. (And we
> also need a better commit message, or at least a proposal.)
>
> This'd probably also help when investigating the extra byte issue,
> discussed yesterday. (Assuming it's not related to the invalid access
> reported by valgrind / asan).
Due to the sanitizer changes, and this feedback, I'm marking the entry as
waiting on author.
Greetings,
Andres Freund