From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
Cc: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Gasper Zejn <zejn(at)owca(dot)info>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: pglz performance |
Date: | 2019-11-27 08:01:47 |
Message-ID: | 20191127080147.GB221153@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Nov 26, 2019 at 09:05:59PM +0100, Tomas Vondra wrote:
> Yeah, although the difference is minimal. We could probably construct a
> benchmark where #2 wins, but I think these queries are fairly realistic.
> So I'd just go with #1.
Nice results. Using your benchmarks it indeed looks like patch 1 is a
winner here.
> Code-wise I think the patches are mostly fine, although the comments
> might need some proof-reading.
>
> 1) I wasn't really sure what a "nibble" is, but maybe it's just me and
> it's a well-known term.
>
> 2) First byte use lower -> First byte uses lower
>
> 3) nibble contain upper -> nibble contains upper
>
> 4) to preven possible uncertanity -> to prevent possible uncertainty
>
> 5) I think we should briefly explain why memmove would be incompatible
> with pglz, it's not quite clear to me.
>
> 6) I'm pretty sure the comment in the 'while (off < len)' branch will be
> badly mangled by pgindent.
>
> 7) The last change moving "copy" to the next line seems unnecessary.
Patch 1 has a typo as well here:
+ * When offset is smaller than lengh - source and
s/lengh/length/
Okay, if we are reaching a conclusion here, Tomas or Peter, are you
planning to finish brushing the patch and potentially commit it?
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2019-11-27 08:05:08 | Re: Psql patch to show access methods info |
Previous Message | Pavel Stehule | 2019-11-27 07:47:56 | missing estimation for coalesce function |