Greetings, * Paul Ramsey (pram...@cleverelephant.ca) wrote: > > On Mar 19, 2019, at 4:47 AM, Stephen Frost <sfr...@snowman.net> wrote: > > * Paul Ramsey (pram...@cleverelephant.ca) wrote: > >>> On Mar 18, 2019, at 7:34 AM, Robert Haas <robertmh...@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, since we cannot be sure that the > size of the uncompressed bit is as large as the requested slice (even though > it will be 99 times 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 not sure we ever hit. Instead of > checking that both the src and dst buffers are fully used up, a test that at > least one of them 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
signature.asc
Description: PGP signature
