Hi!
21 февр. 2019 г., в 23:50, Paul Ramsey <pramsey@cleverelephant.ca> написал(а):
Merci! Attached are updated patches.
<compressed-datum-slicing-20190221a.patch><compressed-datum-slicing-left-20190221a.patch>
As noted before, patches are extremely useful.
So, I've looked into the code too.
I've got some questions about pglz_decompress() changes:
1.
+
if (dp >= destend)
/* check for buffer overrun */
+
break;
/* do not clobber memory */
This is done inside byte-loop. But can we just calculate len = min(len, destend - dp) beforehand?
2. Function argument is_slice is only preventing error.
+
* If we are slicing, then we won't necessarily
+
* be at the end of the source or dest buffers
+
* when we hit a stop, so we don't test then.
But I do not get why we should get that error. If we have limited dest, why we should not fill it entirely?
Wow, took some sleuthing to figure out why this has to be the way that it is…
In the current code, there’s no true slicing, at some point all the slicing logic flows down into toast_decompress_datum() which knows via TOAST_COMPRESS_RAWSIZE() [3] exactly how big the original object was.
In the new code, we have to deal with a slice, which isn’t always exactly sized to the expected output. For example, the text_substring() function only knows it will have N *characters* [2], which it turns into a slice size of N*max_character_encoding_size (effectively N*4 for UTF-8). We could use the TOAST_COMPRESS_RAWSIZE to know the exact size of the entire original object and only pass in a buffer of that size, but we cannot know the size of the slice. It could be 4 chars of English and 4 bytes, or 4 chars of Chinese and 16 bytes. Once you stop using TOAST_COMPRESS_RAWSIZE to exactly size your destination buffer, it’s possible (and it does happen) to have a destination buffer length passed into pglz_decompress that is much larger than the actual contents of the decompressed data, so we end up in a stopping condition in which we’ve run out of source buffer but still have plenty of destination buffer available to fill (illegal in the current code [1]
Anyways, all that to say, we can count on one of sp == srcend (had to decompress the whole thing) or dp == dstend (filled buffer before running out of source data) being true at the end of a slicing decompression, but not both at the same time. The only time they are going to be true at the same time is if we know a priori the size of the output, which we don’t, always (as in text_substring, where we only know the upper bounded size of the output).
Sorry, lot of words there. Anyways, we can still retain the old signature if that’s a deal breaker.
P.
