On Wed, Nov 27, 2019 at 05:01:47PM +0900, Michael Paquier wrote:
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?

I'd like some feedback from Andrey regarding the results and memmove
comment, ant then I'll polish and push.

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply via email to