Re: Dead code in toast_fetch_datum_slice?

Greetings,

* Paul Ramsey (pramsey(at)cleverelephant(dot)ca) wrote:
> 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.

Thanks for taking a look.

For the archives sake, this has now been committed.

Thanks again!

Stephen