Re: Dead code in toast_fetch_datum_slice?

On Fri, Dec 7, 2018 at 3:25 PM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
>
> Perhaps I'm missing something, but in toast_fetch_datum_slice() there's:
>
> Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));
>
> Further, the only caller of this function today is
> heap_tuple_untoast_attr_slice(), which does:
>
> /* fast path for non-compressed external datums */
> if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
> return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
>
> As such, I'm feeling like that conditional to handle the case where this
> function is passed a compressed TOAST value is rather confusing dead
> code.
>
> Hence I'm proposing the attached

Your analysis looks correct to me, I'm pretty sure I had the same reaction,
first time I read through. It would be nice to handle partial decompression
all the way down at this level, but unfortunately the comment at the
Assert() is right: there's no way to know how many of the toasted pieces
need to be read in order to have enough compressed input to create the
desired amount of decompressed output, so there's no choice except to read
the whole compressed thing, even in a slicing context.

P.