Re: ALTER TABLE uses a bistate but not for toast tables

On Tue, Nov 19, 2024 at 03:45:19PM +0100, Dmitry Dolgov wrote:
> > On Mon, Jul 15, 2024 at 03:43:24PM GMT, Justin Pryzby wrote:
> > @cfbot: rebased
>
> Thanks for rebasing. To help with review, could you also describe
> current status of the patch? I have to admit, currently the commit
> message doesn't tell much, and looks more like notes for the future you.

The patch does what it aims to do and AFAIK in a reasonable way. I'm
not aware of any issue with it. It's, uh, waiting for review.

I'm happy to expand on the message to describe something like design
choices, but the goal here is really simple: why should wide column
values escape the intention of the ring buffer? AFAICT it's fixing an
omission. If you have a question, please ask; that would help to
indicate what needs to be explained.

> The patch numbering is somewhat confusing as well, should it be v5 now?

The filename was 0001-WIP-use-BulkInsertState-for-toast-tuples-too.patch.
I guess you're referring to the previous filename: v4-*.
That shouldn't be so confusing -- I just didn't specify a version,
either by choice or by omission.

> From what I understand, the new patch does address the review feedback,
> but you want to do more, something with copy to / copy from?

If I were to do more, it'd be for a future patch, if the current patch
were to ever progress.

> Since it's in the performance category, I'm also curious how much
> overhead does this shave off? I mean, I get it that bulk insert strategy
> helps with buffers usage, as you've implied in the thread -- but how
> does it look like in benchmark numbers?

The intent of using a bistate isn't to help the performance of the
process using the bistate. Rather, the intent is to avoid harming the
performance of other processes. If anything, I expect it could slow
down the process using bistate -- the same as for non-toast data.

https://www.postgresql.org/message-id/CA%2BTgmobC6RD2N8kbPPTvATpUY1kisY2wJLh2jsg%3DHGoCp2RiXw%40mail.gmail.com

--
Justin