Greg,
* Greg Smith (greg@2ndQuadrant.com) wrote:
> That seems easy enough to do here, Heikki's test script is
> excellent. The latest patch Hari posted on July 2 has one hunk that
> doesn't apply anymore now. Inside
> src/backend/utils/adt/pg_lzcompress.c the patch tries to change this
> code:
>
> - if (hent)
> + if (hentno != INVALID_ENTRY)
hentno certainly doesn't make much sense here- it's only used at the top
of the function to keep things a bit cleaner when extracting the address
into hent from hist_entries:
hentno = hstart[pglz_hist_idx(input, end, mask)]; hent = &hist_entries[hentno];
Indeed, as the referenced conditional is inside the following loop:
while (hent != INVALID_ENTRY_PTR)
and, since hentno == 0 implies hent == INVALID_ENTRY_PTR, the
conditional would never fail (which is what was happening prior to
Heikki commiting the fix for this, changing the conditional to what is
below).
> But that line looks like this now:
>
> if (hent != INVALID_ENTRY_PTR)
Right, this is correct- it's useful to check the new value for hent
after it's been updated by:
hent = hent->next;
and see if it's possible to drop out early.
> I'm not sure if different error handling may be needed here now due
> the commit that changed this, or if the patch wasn't referring to
> the right type of error originally.
I've not looked at anything regarding this beyond this email, but I'm
pretty confident that the change Heikki committed was the correct one.
Thanks,
Stephen