Message-ID: <d2e3ef5c-ac09-4b10-a99f-4c964b490f8c@enterprisedb.com>
Date: Sun, 3 Mar 2024 21:41:18 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Shared detoast Datum proposal
Content-Language: en-US
To: Andy Fan <zhihuifan1213@163.com>
Cc: Nikita Malakhov <hukutoc@gmail.com>,
 Michael Zhilin <m.zhilin@postgrespro.ru>, Peter Smith
 <smithpb2250@gmail.com>, vignesh C <vignesh21@gmail.com>,
 Matthias van de Meent <boekewurm+postgres@gmail.com>,
 pgsql-hackers@lists.postgresql.org
References: <87il4jrk1l.fsf@163.com>
 <CALDaNm2EtgXGd+w81ZjTZBD_rLSURyGqXRoz7EFfhxEneOmWig@mail.gmail.com>
 <87sf3andy2.fsf@163.com> <87wmskm9qu.fsf@163.com>
 <CAHut+PsehMtwQ4uE9VS2C7jB1yT0SZVOPU22u1=ma1K7dKFUow@mail.gmail.com>
 <87v87kvboq.fsf@163.com>
 <71321d83-391b-473b-aaf6-618c7f682b96@postgrespro.ru>
 <87r0i7vos3.fsf@163.com> <874je3hcwn.fsf@163.com>
 <c066e988-38b9-460f-9e9d-aedd2482bff2@enterprisedb.com>
 <8734tmg88e.fsf@163.com>
 <7f4c8673-4c5d-4dbf-b8e5-0ec11cd2fba0@enterprisedb.com>
 <87h6i2djf9.fsf@163.com> <87jzms0xza.fsf@163.com>
 <CAN-LCVMU_AzYNDQ-MgX3xc5qCnhTgO1LWKUCygOu3neBM-y94g@mail.gmail.com>
 <87frxf1gld.fsf@163.com>
 <fea81484-09b5-499d-80d0-acb8df43d047@enterprisedb.com>
 <CAN-LCVPv1f0DKk9UvpJ_OnY6CcpNvo7M-RkZVfLi7xRGdBNjqQ@mail.gmail.com>
 <871q8z19ux.fsf@163.com>
 <CAN-LCVON0vAr6tyiK4jsLmkCPd6PxkfxzXDyG8rjM9t3jOVM6g@mail.gmail.com>
 <87msrgdq93.fsf@163.com> <87frx7et1j.fsf@163.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <87frx7et1j.fsf@163.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/d2e3ef5c-ac09-4b10-a99f-4c964b490f8c%40enterprisedb.com>
Precedence: bulk

On 3/3/24 07:10, Andy Fan wrote:
> 
> Hi,
> 
> Here is a updated version, the main changes are:
> 
> 1. an shared_detoast_datum.org file which shows the latest desgin and
> pending items during discussion. 
> 2. I removed the slot->pre_detoast_attrs totally.
> 3. handle some pg_detoast_datum_slice use case.
> 4. Some implementation improvement.
> 

I only very briefly skimmed the patch, and I guess most of my earlier
comments still apply. But I'm a bit surprised the patch needs to pass a
MemoryContext to so many places as a function argument - that seems to
go against how we work with memory contexts. Doubly so because it seems
to only ever pass CurrentMemoryContext anyway. So what's that about?

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company