Date: Tue, 15 Jul 2025 16:54:13 +0900 (JST)
Message-Id: <20250715.165413.1502624024677287358.kou@clear-code.com>
To: sawada.mshk@gmail.com
Cc: michael@paquier.xyz, david.g.johnston@gmail.com, tgl@sss.pgh.pa.us,
 zhjwpku@gmail.com, pgsql-hackers@postgresql.org
Subject: Re: Make COPY format extendable: Extract COPY TO format
 implementations
From: Sutou Kouhei <kou@clear-code.com>
In-Reply-To: <20250714.173803.865595983884510428.kou@clear-code.com>
References: 
 <CAD21AoCyTcYzFocKhtpWMBp0V27d3sLpwnrbu=HaDSOHMxSFXg@mail.gmail.com>
	<CAD21AoB0Z3gkOGALK3pXYmGTWATVvgDAmn-yXGp2mX64S-YrSw@mail.gmail.com>
	<20250714.173803.865595983884510428.kou@clear-code.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20250715.165413.1502624024677287358.kou%40clear-code.com>
Precedence: bulk

Hi,

In <20250714.173803.865595983884510428.kou@clear-code.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 14 Jul 2025 17:38:03 +0900 (JST),
  Sutou Kouhei <kou@clear-code.com> wrote:

>> I've reviewed the 0001 and 0002 patches. The API implemented in the
>> 0002 patch looks good to me, but I'm concerned about the capsulation
>> of copy state data. With the v42 patches, we pass the whole
>> CopyToStateData to the extension codes, but most of the fields in
>> CopyToStateData are internal working state data that shouldn't be
>> exposed to extensions. I think we need to sort out which fields are
>> exposed or not. That way, it would be safer and we would be able to
>> avoid exposing copyto_internal.h and extensions would not need to
>> include copyfrom_internal.h.

> In general, I agree that we should export only needed
> information.
> 
> How about adding accessors instead of splitting
> Copy{From,To}State to Copy{From,To}ExecutionData? If we use
> the accessors approach, we can export only needed
> information step by step without breaking ABI.

Another idea: We'll add Copy{From,To}State::opaque
eventually. (For example, the v40-0003 patch includes it.)

How about using it to hide fields only for built-in formats?


Thanks,
-- 
kou