Greetings,
* Paul Ramsey (pramsey@cleverelephant.ca) wrote:
> > On Mar 19, 2019, at 4:47 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > * Paul Ramsey (pramsey@cleverelephant.ca) wrote:
> >>> On Mar 18, 2019, at 7:34 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> >>> +1. I think Paul had it right originally.
> >>
> >> In that spirit, here is a “one pglz_decompress function, new parameter” version for commit.
> >
> > Alright, I've been working through this and have made a few improvements
> > (the big comment block at the top of pg_lzcompress.c needed updating,
> > among a couple other minor things), but I was trying to wrap my head
> > around this:
> >
> >
> > Specifically, the two SET_VARSIZE() calls, do we really need both..?
> > Are we sure that we're setting the length correctly there..? Is there
> > any cross-check we can do?
>
> Well, we don’t need to do the two SET_VARSIZE() calls, but we *do* need to use rawsize in the call before the return,
sincewe cannot be sure that the size of the uncompressed bit is as large as the requested slice (even though it will be
99times out of 100)
Sure, of course, that makes sense, what didn't make much sense was
setting it and then setting it again to something different.
I'll pull out the extra one then.
> > I have to admit that I find the new argument to pglz_decompress() a bit
> > awkward to describe and document; if you have any thoughts as to how
> > that could be improved, that'd be great.
>
> The only thing I can see is loosening the integrity check in pglz_decompress which is a guardrail on something I’m
notsure we ever hit. Instead of checking that both the src and dst buffers are fully used up, a test that at least one
ofthem is used up should come up true in all error-free-happy cases.
Hrmpf. I don't really like loosening up the integrity check in the
cases where we should be using up everything though. As such, I'll go
with what you've proposed here. We can adjust it later if we end up
deciding that reducing the error-checking is reasonable.
I'll plan to push this tomorrow with the above change (and a few
additional comments to explain what all is going on..).
Thanks!
Stephen