Hi,
I took a look at the v6 patch, with the intention to get it committed. I
have a couple minor comments:
1) For PGLZ_HISTORY_SIZE it uses literal 0x0fff, with the explanation:
/* to avoid compare in iteration */
which I think means intent to use this value as a bit mask, but then the
only place using PGLZ_HISTORY_SIZE does
if (hist_next == PGLZ_HISTORY_SIZE) ...
i.e. a comparison. Maybe I misunderstand the comment, though.
2) PGLZ_HistEntry was modified and replaces links (pointers) with
indexes, but the comments still talk about "links", so maybe that needs
to be changed. Also, I wonder why next_id is int16 while hist_idx is
uint16 (and also why id vs. idx)?
3) minor formatting of comments
4) the comment in pglz_find_match about traversing the history seems too
early - it's before handling invalid entries and cleanup, but it does
not talk about that at all, and the actual while loop is after that.
Attached is v6 in 0001 (verbatim), with the review comments in 0002.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
