Date: Mon, 29 Jan 2024 15:03:32 +0900 (JST)
Message-Id: <20240129.150332.225219562175209481.kou@clear-code.com>
To: zhjwpku@gmail.com
Cc: sawada.mshk@gmail.com, andrew@dunslane.net, michael@paquier.xyz,
 nathandbossart@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: 
 <CAEG8a3JDPks7XU5-NvzjzuKQYQqR8pDfS7CDGZonQTXfdWtnnw@mail.gmail.com>
References: <20240126.175511.2017689682651924758.kou@clear-code.com>
	<CAEG8a3L86MHHu=DFqS-+ye05nH-dqNGxdojLh-CyFhUeCHZY7g@mail.gmail.com>
	<CAEG8a3JDPks7XU5-NvzjzuKQYQqR8pDfS7CDGZonQTXfdWtnnw@mail.gmail.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20240129.150332.225219562175209481.kou%40clear-code.com>
Precedence: bulk

Hi,

In <CAEG8a3JDPks7XU5-NvzjzuKQYQqR8pDfS7CDGZonQTXfdWtnnw@mail.gmail.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Sat, 27 Jan 2024 14:15:02 +0800,
  Junwang Zhao <zhjwpku@gmail.com> wrote:

> I have been working on a *COPY TO JSON* extension since yesterday,
> which is based on your V6 patch set, I'd like to give you more input
> so you can make better decisions about the implementation(with only
> pg-copy-arrow you might not get everything considered).

Thanks!

> 0009 is some changes made by me, I changed CopyToGetFormat to
> CopyToSendCopyBegin because pg_copy_json need to send different bytes
> in SendCopyBegin, get the format code along is not enough

Oh, I haven't cared about the case.
How about the following API instead?

static void
SendCopyBegin(CopyToState cstate)
{
	StringInfoData buf;

	pq_beginmessage(&buf, PqMsg_CopyOutResponse);
	cstate->opts.to_routine->CopyToFillCopyOutResponse(cstate, &buf);
	pq_endmessage(&buf);
	cstate->copy_dest = COPY_FRONTEND;
}

static void
CopyToJsonFillCopyOutResponse(CopyToState cstate, StringInfoData &buf)
{
	int16		format = 0;

	pq_sendbyte(&buf, format);      /* overall format */
	/*
	 * JSON mode is always one non-binary column
	 */
	pq_sendint16(&buf, 1);
	pq_sendint16(&buf, format);
}

> 00010 is the pg_copy_json extension, I think this should be a good
> case which can utilize the *extendable copy format* feature

It seems that it's convenient that we have one more callback
for initializing CopyToState::opaque. It's called only once
when Copy{To,From}Routine is chosen:

typedef struct CopyToRoutine
{
	void		(*CopyToInit) (CopyToState cstate);
...
};

void
ProcessCopyOptions(ParseState *pstate,
				   CopyFormatOptions *opts_out,
				   bool is_from,
				   void *cstate,
				   List *options)
{
...
	foreach(option, options)
	{
		DefElem    *defel = lfirst_node(DefElem, option);

		if (strcmp(defel->defname, "format") == 0)
		{
			...
			opts_out->to_routine = &CopyToRoutineXXX;
			opts_out->to_routine->CopyToInit(cstate);
			...
		}
	}
...
}


>                                                              maybe we
> should delete copy_test_format if we have this extension as an
> example?

I haven't read the COPY TO format json thread[1] carefully
(sorry), but we may add the JSON format as a built-in
format. If we do it, copy_test_format is useful to test the
extension API.

[1] https://www.postgresql.org/message-id/flat/CALvfUkBxTYy5uWPFVwpk_7ii2zgT07t3d-yR_cy4sfrrLU%3Dkcg%40mail.gmail.com


Thanks,
-- 
kou