Date: Thu, 14 Aug 2025 15:36:54 +0900 (JST)
Message-Id: <20250814.153654.91221343186154858.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: 
 <CAD21AoBa0Wm3C2H12jaqkvLidP2zEhsC+gf=3w7XiA4LQnvx0g@mail.gmail.com>
References: <20250718.190553.1172585000083080334.kou@clear-code.com>
	<CAD21AoB01+FZ_bOR4uOq_x_qBE8on1ASdJWm+UexxOwC1RGD-A@mail.gmail.com>
	<CAD21AoBa0Wm3C2H12jaqkvLidP2zEhsC+gf=3w7XiA4LQnvx0g@mail.gmail.com>
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Thu_Aug_14_15_36_54_2025_549)--"
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20250814.153654.91221343186154858.kou%40clear-code.com>
Precedence: bulk

----Next_Part(Thu_Aug_14_15_36_54_2025_549)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,

In <CAD21AoBa0Wm3C2H12jaqkvLidP2zEhsC+gf=3w7XiA4LQnvx0g@mail.gmail.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 28 Jul 2025 22:19:36 -0700,
  Masahiko Sawada <sawada.mshk@gmail.com> wrote:

> The fields in 1 are mostly static fields, and the fields in 2 and 3
> are likely to be accessed in hot functions during COPY FROM. Would it
> be a good idea to restructure these fields so that we can hide the
> fields in 1 from callback functions and having the fields in 3 in a
> separate format-specific struct that can be accessed via an opaque
> pointer? But could the latter change potentially cause performance
> overheads?

Yes. It changes memory layout (1 continuous memory chunk ->
2 separated memory chunks) and introduces indirect member
accesses (x->y -> x->z->y). They may not have performance
impact but we need to measure it if we want to use this
approach.


BTW, how about the following approach?

copyapi.h:

typedef struct CopyToStateData
{
	/* public members */
	/* ... */
} CopyToStateData;

copyto.c:

typedef struct CopyToStateInternalData
{
	CopyToStateData parent;

	/* private members */
	/* ... */
} CopyToStateInternalData;

We export CopyToStateData only with public members. We don't
export CopyToStateInternalData that has members only for
built-in formats.

CopyToStateInternalData has the same memory layout as
CopyToStateData. So we can use CopyToStateInternalData as
CopyToStateData.

We use CopyToStateData not CopyToStateInternalData in public
API. We cast CopyToStateData to CopyToStateInternalData when
we need to use private members:

static void
CopySendData(CopyToState cstate, const void *databuf, int datasize)
{
	CopyToStateInternal cstate_internal = (CopyToStateInternal) cstate;
	appendBinaryStringInfo(cstate_internal->fe_msgbuf, databuf, datasize);
}

It's still direct member access.


With this approach, we can keep the same memory layout (1
continuous memory chunk) and direct member access. I think
that this approach doesn't have performance impact.

See the attached patch for PoC of this approach.

Drawback: This approach always allocates
CopyToStateInternalData not CopyToStateData. So we need to
allocate needless memories for extensions. But this will
prevent performance regression of built-in formats. Is it
acceptable?


Thanks,
-- 
kou

----Next_Part(Thu_Aug_14_15_36_54_2025_549)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0001-Split-CopyToStateData-to-CopyToStateData-and-CopyToS.patch"