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

From: Nikita Malakhov <hukutoc(at)gmail(dot)com>
To: Andy Fan <zhihuifan1213(at)163(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, David Rowley <dgrowley(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>
Subject: Re: detoast datum into the given buffer as a optimization.
Date: 2024-10-30 13:23:01
Message-ID: CAN-LCVM5hiF2Fpaqz8VTdnxnfkQZNF1_OWsGoU9PGM=atKSkpg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!

Andy, I'm truly sorry, I removed quoted messages just to not
make replies to walls of text.

On detoast_attr_buffer topic - and again agree with reply above
that supposes adding required data length as parameter, at least
by doing so we should explicitly make the user of this API pay
attention to the length of the buffer and data size, as it is done
in text_to_cstring_buffer you mentioned.

About printtup: I agree direct detoasting to the buffer could give
us some performance improvement, it seems not too difficult
to check, but as you can see there is a function call via fmgr,
it is better to try to find out why such a decision was made.

On Wed, Oct 30, 2024 at 2:47 AM Andy Fan <zhihuifan1213(at)163(dot)com> wrote:

> Nikita Malakhov <hukutoc(at)gmail(dot)com> writes:
>
> > Hi!
> >
> > Sorry for misguiding you, I've overlooked va_rawsize with va_extinfo.
> > You're right, va_rawsize holds uncompressed size, and extinfo actual
> > storage size. This was not intentional.
>
> That's OK, so we are in the same page here.
>
> > I'd better not count on caller's do know detoasted data length,
> > and much more the buffer is correctly initialized, because we cannot
> > check that inside and must rely on the caller, which would for sure
> > lead to unexpected segfaults, I agree with Tom Lane's proposal above.
> > Other options seem to be more crude and error-prone here. This is
> > an internal data fetching function and it should not generate new kinds
> > of errors, I think.
>
> Tom'sreply depends on the fact I was going to changing the "detoast_attr"
> to "detoast_attr_buffer", as I have expalined in my previous post. I
> don't think a [new] user provided buffer function is so harmful. What
> do you think about function "text_to_string_buffer"? This is also a
> part in my previous reply, but is ignored by your reply?
>
> > In case you're playing with this part of the code - I was always
> > confused with detoast_attr and detoast_external_attr functions
> > both marked as entry points of retrieving toasted data and both
> > look a lot the same. Have you ever thought of making a single
> > entry point by slightly redesigning this part?
>
> You can check [1] for a indepent improvements for this topic.
>
> [1] https://www.postgresql.org/message-id/874j4vcspl.fsf%40163.com
>
> --
> Best Regards
> Andy Fan
>
>
> --
Regards,
Nikita Malakhov
Postgres Professional
The Russian Postgres Company
https://postgrespro.ru/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2024-10-30 13:29:00 Re: general purpose array_sort
Previous Message Robert Haas 2024-10-30 13:09:37 Re: Eager aggregation, take 3