Re: detoast datum into the given buffer as a optimization.

Hi Tom,

> Andy Fan <zhihuifan1213(at)163(dot)com> writes:
>> * Note if caller provides a non-NULL buffer, it is the duty of caller
>> * to make sure it has enough room for the detoasted format (Usually
>> * they can use toast_raw_datum_size to get the size)
>
> This is a pretty awful, unsafe API design.

Sorry that I expressed my thoughts incorrectly. I'm not going to
refactor "detoast_attr", I'm going to add a new API named
"detoast_attr_buffer", which very similar with text_to_cstring and
text_to_string_buffer. Most of the user still can using detoast_attr,
only the user who care about the MemoryContext or memcpy issue, they can
the deotast_attr_buffer variant.

> It puts it on the caller
> to know how to get the detoasted length, and it implies double
> decoding of the toast datum.

I really nearly give up this idea today because of this, later I
realized something which was known when I was writing the first message
is forgotten by me now. Since it is really confused, I want to highlight it
here for double check from more people.

I thought 'toast_raw_datum_size' is the existing function to get the
"detoasted length" for the caller of detoast_attr_buffer, so it looks
reasonable for me to assume the caller knows how to get the detoast
length.

What really confused me suddenly is it is really correct to use
"toast_raw_datum_size". In "toast_raw_datum_size":

if (VARATT_IS_EXTERNAL_ONDISK(attr))
{
/* va_rawsize is the size of the original datum -- including header */
struct varatt_external toast_pointer;

VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
result = toast_pointer.va_rawsize;
}

We just return the va_rawsize directly. Does it work for a datum which
is compressed first and then stored on external ondisk? After some
more research, it is correct since the rawsize is the size of
uncompressed data. and we also use the fact in
1b393f4e5db4fd6bbc86a4e88785b6945a1541d0. This is why I said:

> One of the key point is we can always get the varlena rawsize cheaply
> without any real detoast activity in advance, thanks to the existing
> varlena design.

This is the thing I forget today.

Since user can know the size easily now, is it cheap to use it? By
looking the code in toast_raw_datum_size, I really hard to say it is
expensive.

>
> How about a variant like
>
> struct varlena *
> detoast_attr_cxt(struct varlena *attr, MemoryContext cxt)
>
> which promises to allocate the result in the specified context?
> That would cover most of the practical use-cases, I think.

Yes, it work for some use case, and it is similar with what I did in
[1] (search detoast_attr_ext). However it can't support the case where
user want to detoast the data into the given buffer (to avoid the later
memcpy), so detoast_attr_buffer is the favorite API right now. If it is
not doable, detoast_attr_ctx is also works for me.

Would you still think detoast_attr_buffer is not a acceptable API now at
the high level design. I'm working on an implementation, but I want have
some high-level designment agreement first.

[1]
https://www.postgresql.org/message-id/attachment/160491/v10-0001-shared-detoast-datum.patch

--
Best Regards
Andy Fan