public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH 2/7] Write WAL for empty nbtree index build
21+ messages / 5 participants
[nested] [flat]

* [PATCH 2/7] Write WAL for empty nbtree index build
@ 2018-10-11 01:03  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Kyotaro Horiguchi @ 2018-10-11 01:03 UTC (permalink / raw)

After relation truncation indexes are also rebuild. It doesn't emit
WAL in minimal mode and if truncation happened within its creation
transaction, crash recovery leaves an empty index heap, which is
considered broken. This patch forces to emit WAL when an index_build
turns into empty nbtree index.
---
 src/backend/access/nbtree/nbtsort.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 14d9545768..5551a9c227 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -622,8 +622,16 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
 	/* Ensure rd_smgr is open (could have been closed by relcache flush!) */
 	RelationOpenSmgr(wstate->index);
 
-	/* XLOG stuff */
-	if (wstate->btws_use_wal)
+	/* XLOG stuff
+	 *
+	 * Even when wal_level is minimal, WAL is required here if truncation
+	 * happened after being created in the same transaction. This is hacky but
+	 * we cannot use BufferNeedsWAL() stuff for nbtree since it can emit
+	 * atomic WAL records on multiple buffers.
+	 */
+	if (wstate->btws_use_wal ||
+		(RelationNeedsWAL(wstate->index) &&
+		 (blkno == BTREE_METAPAGE && BTPageGetMeta(page)->btm_root == 0)))
 	{
 		/* We use the heap NEWPAGE record type for this */
 		log_newpage(&wstate->index->rd_node, MAIN_FORKNUM, blkno, page, true);
-- 
2.16.3


----Next_Part(Tue_Apr_02_19_54_06_2019_801)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v10-0003-Move-XLOG-stuff-from-heap_insert-and-heap_delete.patch"



^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH 2/8] Write WAL for empty nbtree index build
@ 2018-10-11 01:03  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Kyotaro Horiguchi @ 2018-10-11 01:03 UTC (permalink / raw)

After relation truncation indexes are also rebuild. It doesn't emit
WAL in minimal mode and if truncation happened within its creation
transaction, crash recovery leaves an empty index heap, which is
considered broken. This patch forces to emit WAL when an index_build
turns into empty nbtree index.
---
 src/backend/access/nbtree/nbtsort.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 46e0831834..e65d4aab0f 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -622,8 +622,15 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
 	/* Ensure rd_smgr is open (could have been closed by relcache flush!) */
 	RelationOpenSmgr(wstate->index);
 
-	/* XLOG stuff */
-	if (wstate->btws_use_wal)
+	/* XLOG stuff
+	 *
+	 * Even if minimal mode, WAL is required here if truncation happened after
+	 * being created in the same transaction. It is not needed otherwise but
+	 * we don't bother identifying the case precisely.
+	 */
+	if (wstate->btws_use_wal ||
+		(RelationNeedsWAL(wstate->index) &&
+		 (blkno == BTREE_METAPAGE && BTPageGetMeta(page)->btm_root == 0)))
 	{
 		/* We use the heap NEWPAGE record type for this */
 		log_newpage(&wstate->index->rd_node, MAIN_FORKNUM, blkno, page, true);
-- 
2.16.3


----Next_Part(Tue_Mar_26_16_35_07_2019_128)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v9-0003-Move-XLOG-stuff-from-heap_insert-and-heap_delete.patch"



^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH 2/7] Write WAL for empty nbtree index build
@ 2018-10-11 01:03  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Kyotaro Horiguchi @ 2018-10-11 01:03 UTC (permalink / raw)

After relation truncation indexes are also rebuild. It doesn't emit
WAL in minimal mode and if truncation happened within its creation
transaction, crash recovery leaves an empty index heap, which is
considered broken. This patch forces to emit WAL when an index_build
turns into empty nbtree index.
---
 src/backend/access/nbtree/nbtsort.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 2762a2d548..70fe3bec32 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -622,8 +622,15 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
 	/* Ensure rd_smgr is open (could have been closed by relcache flush!) */
 	RelationOpenSmgr(wstate->index);
 
-	/* XLOG stuff */
-	if (wstate->btws_use_wal)
+	/* XLOG stuff
+	 *
+	 * Even if minimal mode, WAL is required here if truncation happened after
+	 * being created in the same transaction. It is not needed otherwise but
+	 * we don't bother identifying the case precisely.
+	 */
+	if (wstate->btws_use_wal ||
+		(RelationNeedsWAL(wstate->index) &&
+		 (blkno == BTREE_METAPAGE && BTPageGetMeta(page)->btm_root == 0)))
 	{
 		/* We use the heap NEWPAGE record type for this */
 		log_newpage(&wstate->index->rd_node, MAIN_FORKNUM, blkno, page, true);
-- 
2.16.3


----Next_Part(Mon_Mar_25_21_32_04_2019_838)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v8-0003-Move-XLOG-stuff-from-heap_insert-and-heap_delete.patch"



^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH 2/4] Write WAL for empty nbtree index build
@ 2018-10-11 01:03  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Kyotaro Horiguchi @ 2018-10-11 01:03 UTC (permalink / raw)

After relation truncation indexes are also rebuild. It doesn't emit
WAL in minimal mode and if truncation happened within its creation
transaction, crash recovery leaves an empty index heap, which is
considered broken. This patch forces to emit WAL when an index_build
turns into empty nbtree index.
---
 src/backend/access/nbtree/nbtsort.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index dc398e1186..70d4380533 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -611,8 +611,14 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
 	/* Ensure rd_smgr is open (could have been closed by relcache flush!) */
 	RelationOpenSmgr(wstate->index);
 
-	/* XLOG stuff */
-	if (wstate->btws_use_wal)
+	/* XLOG stuff
+	 *
+	 * Even if minimal mode, WAL is required here if truncation happened after
+	 * being created in the same transaction. It is not needed otherwise but
+	 * we don't bother identifying the case precisely.
+	 */
+	if (wstate->btws_use_wal ||
+		(blkno == BTREE_METAPAGE && BTPageGetMeta(page)->btm_root == 0))
 	{
 		/* We use the heap NEWPAGE record type for this */
 		log_newpage(&wstate->index->rd_node, MAIN_FORKNUM, blkno, page, true);
@@ -1056,6 +1062,11 @@ _bt_uppershutdown(BTWriteState *wstate, BTPageState *state)
 	 * set to point to "P_NONE").  This changes the index to the "valid" state
 	 * by filling in a valid magic number in the metapage.
 	 */
+	/*
+	 * If no tuple was inserted, it's possible that we are truncating a
+	 * relation. We need to emit WAL for the metapage in the case. However it
+	 * is not required elsewise,
+	 */
 	metapage = (Page) palloc(BLCKSZ);
 	_bt_initmetapage(metapage, rootblkno, rootlevel);
 	_bt_blwritepage(wstate, metapage, BTREE_METAPAGE);
-- 
2.16.3


----Next_Part(Mon_Mar_04_12_24_48_2019_788)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v7-0003-Add-infrastructure-to-WAL-logging-skip-feature.patch"



^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH 2/7] Write WAL for empty nbtree index build
@ 2018-10-11 01:03  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Kyotaro Horiguchi @ 2018-10-11 01:03 UTC (permalink / raw)

After relation truncation indexes are also rebuild. It doesn't emit
WAL in minimal mode and if truncation happened within its creation
transaction, crash recovery leaves an empty index heap, which is
considered broken. This patch forces to emit WAL when an index_build
turns into empty nbtree index.
---
 src/backend/access/nbtree/nbtsort.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 9ac4c1e1c0..a31d58025f 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -654,8 +654,16 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
 	/* Ensure rd_smgr is open (could have been closed by relcache flush!) */
 	RelationOpenSmgr(wstate->index);
 
-	/* XLOG stuff */
-	if (wstate->btws_use_wal)
+	/* XLOG stuff
+	 *
+	 * Even when wal_level is minimal, WAL is required here if truncation
+	 * happened after being created in the same transaction. This is hacky but
+	 * we cannot use BufferNeedsWAL() stuff for nbtree since it can emit
+	 * atomic WAL records on multiple buffers.
+	 */
+	if (wstate->btws_use_wal ||
+		(RelationNeedsWAL(wstate->index) &&
+		 (blkno == BTREE_METAPAGE && BTPageGetMeta(page)->btm_root == 0)))
 	{
 		/* We use the heap NEWPAGE record type for this */
 		log_newpage(&wstate->index->rd_node, MAIN_FORKNUM, blkno, page, true);
-- 
2.16.3


----Next_Part(Fri_Apr_05_12_55_20_2019_986)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v11-0003-Move-XLOG-stuff-from-heap_insert-and-heap_delete.patch"



^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v16] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations. There are many diffs for CopyOneRowTo() but they're
caused by indentation. They don't change implementations.
---
 src/backend/commands/copyfrom.c          |  24 +++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            | 103 ++++++++++++++---------
 src/include/commands/copyapi.h           | 100 ++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 193 insertions(+), 45 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
-		fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..6080627c83 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,14 +781,22 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
-		fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
 	}
 
 	/*
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,61 +932,66 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
-	if (cstate->opts.binary)
+	if (cstate->routine)
+		cstate->routine->CopyToOneRow(cstate, slot);
+	else
 	{
-		/* Binary per-tuple header */
-		CopySendInt16(cstate, list_length(cstate->attnumlist));
-	}
-
-	/* Make sure the tuple is fully deconstructed */
-	slot_getallattrs(slot);
-
-	foreach(cur, cstate->attnumlist)
-	{
-		int			attnum = lfirst_int(cur);
-		Datum		value = slot->tts_values[attnum - 1];
-		bool		isnull = slot->tts_isnull[attnum - 1];
-
-		if (!cstate->opts.binary)
+		if (cstate->opts.binary)
 		{
-			if (need_delim)
-				CopySendChar(cstate, cstate->opts.delim[0]);
-			need_delim = true;
+			/* Binary per-tuple header */
+			CopySendInt16(cstate, list_length(cstate->attnumlist));
 		}
 
-		if (isnull)
-		{
-			if (!cstate->opts.binary)
-				CopySendString(cstate, cstate->opts.null_print_client);
-			else
-				CopySendInt32(cstate, -1);
-		}
-		else
+		/* Make sure the tuple is fully deconstructed */
+		slot_getallattrs(slot);
+
+		foreach(cur, cstate->attnumlist)
 		{
+			int			attnum = lfirst_int(cur);
+			Datum		value = slot->tts_values[attnum - 1];
+			bool		isnull = slot->tts_isnull[attnum - 1];
+
 			if (!cstate->opts.binary)
 			{
-				string = OutputFunctionCall(&out_functions[attnum - 1],
-											value);
-				if (cstate->opts.csv_mode)
-					CopyAttributeOutCSV(cstate, string,
-										cstate->opts.force_quote_flags[attnum - 1]);
+				if (need_delim)
+					CopySendChar(cstate, cstate->opts.delim[0]);
+				need_delim = true;
+			}
+
+			if (isnull)
+			{
+				if (!cstate->opts.binary)
+					CopySendString(cstate, cstate->opts.null_print_client);
 				else
-					CopyAttributeOutText(cstate, string);
+					CopySendInt32(cstate, -1);
 			}
 			else
 			{
-				bytea	   *outputbytes;
+				if (!cstate->opts.binary)
+				{
+					string = OutputFunctionCall(&out_functions[attnum - 1],
+												value);
+					if (cstate->opts.csv_mode)
+						CopyAttributeOutCSV(cstate, string,
+											cstate->opts.force_quote_flags[attnum - 1]);
+					else
+						CopyAttributeOutText(cstate, string);
+				}
+				else
+				{
+					bytea	   *outputbytes;
 
-				outputbytes = SendFunctionCall(&out_functions[attnum - 1],
-											   value);
-				CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
-				CopySendData(cstate, VARDATA(outputbytes),
-							 VARSIZE(outputbytes) - VARHDRSZ);
+					outputbytes = SendFunctionCall(&out_functions[attnum - 1],
+												   value);
+					CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
+					CopySendData(cstate, VARDATA(outputbytes),
+								 VARSIZE(outputbytes) - VARHDRSZ);
+				}
 			}
 		}
-	}
 
-	CopySendEndOfRow(cstate);
+		CopySendEndOfRow(cstate);
+	}
 
 	MemoryContextSwitchTo(oldcontext);
 }
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Mon_Mar__4_14_11_08_2024_631)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v16-0001-Add-CopyFromRoutine-CopyToRountine-w.patch"



^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v16] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations. There are many diffs for CopyOneRowTo() but they're
caused by indentation. They don't change implementations.
---
 src/backend/commands/copyfrom.c          |  22 ++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            |  21 +++++
 src/include/commands/copyapi.h           | 100 +++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 151 insertions(+), 3 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
 			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..6080627c83 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,15 +781,23 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
 			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
 		}
+	}
 
 	/*
 	 * Create a temporary memory context that we can reset once per row to
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,6 +932,10 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
+	if (cstate->routine)
+		cstate->routine->CopyToOneRow(cstate, slot);
+	else
+	{
 		if (cstate->opts.binary)
 		{
 			/* Binary per-tuple header */
@@ -971,6 +991,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 		}
 
 		CopySendEndOfRow(cstate);
+	}
 
 	MemoryContextSwitchTo(oldcontext);
 }
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Mon_Mar__4_14_11_08_2024_631)----





^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v17] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations.
---
 src/backend/commands/copyfrom.c          |  24 +++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            |  25 +++++-
 src/include/commands/copyapi.h           | 100 +++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 155 insertions(+), 5 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
-		fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..b4a7c9c8b9 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,14 +781,22 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
-		fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
 	}
 
 	/*
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,6 +932,13 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
+	if (cstate->routine)
+	{
+		cstate->routine->CopyToOneRow(cstate, slot);
+		MemoryContextSwitchTo(oldcontext);
+		return;
+	}
+
 	if (cstate->opts.binary)
 	{
 		/* Binary per-tuple header */
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Tue_Mar__5_17_18_08_2024_468)----





^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v16] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations. There are many diffs for CopyOneRowTo() but they're
caused by indentation. They don't change implementations.
---
 src/backend/commands/copyfrom.c          |  22 ++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            |  21 +++++
 src/include/commands/copyapi.h           | 100 +++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 151 insertions(+), 3 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
 			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..6080627c83 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,15 +781,23 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
 			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
 		}
+	}
 
 	/*
 	 * Create a temporary memory context that we can reset once per row to
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,6 +932,10 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
+	if (cstate->routine)
+		cstate->routine->CopyToOneRow(cstate, slot);
+	else
+	{
 		if (cstate->opts.binary)
 		{
 			/* Binary per-tuple header */
@@ -971,6 +991,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 		}
 
 		CopySendEndOfRow(cstate);
+	}
 
 	MemoryContextSwitchTo(oldcontext);
 }
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Mon_Mar__4_14_11_08_2024_631)----





^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v17] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations.
---
 src/backend/commands/copyfrom.c          |  24 +++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            |  25 +++++-
 src/include/commands/copyapi.h           | 100 +++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 155 insertions(+), 5 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
-		fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..b4a7c9c8b9 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,14 +781,22 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
-		fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
 	}
 
 	/*
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,6 +932,13 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
+	if (cstate->routine)
+	{
+		cstate->routine->CopyToOneRow(cstate, slot);
+		MemoryContextSwitchTo(oldcontext);
+		return;
+	}
+
 	if (cstate->opts.binary)
 	{
 		/* Binary per-tuple header */
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Tue_Mar__5_17_18_08_2024_468)----





^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v16] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations. There are many diffs for CopyOneRowTo() but they're
caused by indentation. They don't change implementations.
---
 src/backend/commands/copyfrom.c          |  24 +++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            | 103 ++++++++++++++---------
 src/include/commands/copyapi.h           | 100 ++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 193 insertions(+), 45 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
-		fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..6080627c83 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,14 +781,22 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
-		fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
 	}
 
 	/*
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,61 +932,66 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
-	if (cstate->opts.binary)
+	if (cstate->routine)
+		cstate->routine->CopyToOneRow(cstate, slot);
+	else
 	{
-		/* Binary per-tuple header */
-		CopySendInt16(cstate, list_length(cstate->attnumlist));
-	}
-
-	/* Make sure the tuple is fully deconstructed */
-	slot_getallattrs(slot);
-
-	foreach(cur, cstate->attnumlist)
-	{
-		int			attnum = lfirst_int(cur);
-		Datum		value = slot->tts_values[attnum - 1];
-		bool		isnull = slot->tts_isnull[attnum - 1];
-
-		if (!cstate->opts.binary)
+		if (cstate->opts.binary)
 		{
-			if (need_delim)
-				CopySendChar(cstate, cstate->opts.delim[0]);
-			need_delim = true;
+			/* Binary per-tuple header */
+			CopySendInt16(cstate, list_length(cstate->attnumlist));
 		}
 
-		if (isnull)
-		{
-			if (!cstate->opts.binary)
-				CopySendString(cstate, cstate->opts.null_print_client);
-			else
-				CopySendInt32(cstate, -1);
-		}
-		else
+		/* Make sure the tuple is fully deconstructed */
+		slot_getallattrs(slot);
+
+		foreach(cur, cstate->attnumlist)
 		{
+			int			attnum = lfirst_int(cur);
+			Datum		value = slot->tts_values[attnum - 1];
+			bool		isnull = slot->tts_isnull[attnum - 1];
+
 			if (!cstate->opts.binary)
 			{
-				string = OutputFunctionCall(&out_functions[attnum - 1],
-											value);
-				if (cstate->opts.csv_mode)
-					CopyAttributeOutCSV(cstate, string,
-										cstate->opts.force_quote_flags[attnum - 1]);
+				if (need_delim)
+					CopySendChar(cstate, cstate->opts.delim[0]);
+				need_delim = true;
+			}
+
+			if (isnull)
+			{
+				if (!cstate->opts.binary)
+					CopySendString(cstate, cstate->opts.null_print_client);
 				else
-					CopyAttributeOutText(cstate, string);
+					CopySendInt32(cstate, -1);
 			}
 			else
 			{
-				bytea	   *outputbytes;
+				if (!cstate->opts.binary)
+				{
+					string = OutputFunctionCall(&out_functions[attnum - 1],
+												value);
+					if (cstate->opts.csv_mode)
+						CopyAttributeOutCSV(cstate, string,
+											cstate->opts.force_quote_flags[attnum - 1]);
+					else
+						CopyAttributeOutText(cstate, string);
+				}
+				else
+				{
+					bytea	   *outputbytes;
 
-				outputbytes = SendFunctionCall(&out_functions[attnum - 1],
-											   value);
-				CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
-				CopySendData(cstate, VARDATA(outputbytes),
-							 VARSIZE(outputbytes) - VARHDRSZ);
+					outputbytes = SendFunctionCall(&out_functions[attnum - 1],
+												   value);
+					CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
+					CopySendData(cstate, VARDATA(outputbytes),
+								 VARSIZE(outputbytes) - VARHDRSZ);
+				}
 			}
 		}
-	}
 
-	CopySendEndOfRow(cstate);
+		CopySendEndOfRow(cstate);
+	}
 
 	MemoryContextSwitchTo(oldcontext);
 }
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Mon_Mar__4_14_11_08_2024_631)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v16-0001-Add-CopyFromRoutine-CopyToRountine-w.patch"



^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v16] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations. There are many diffs for CopyOneRowTo() but they're
caused by indentation. They don't change implementations.
---
 src/backend/commands/copyfrom.c          |  22 ++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            |  21 +++++
 src/include/commands/copyapi.h           | 100 +++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 151 insertions(+), 3 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
 			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..6080627c83 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,15 +781,23 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
 			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
 		}
+	}
 
 	/*
 	 * Create a temporary memory context that we can reset once per row to
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,6 +932,10 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
+	if (cstate->routine)
+		cstate->routine->CopyToOneRow(cstate, slot);
+	else
+	{
 		if (cstate->opts.binary)
 		{
 			/* Binary per-tuple header */
@@ -971,6 +991,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 		}
 
 		CopySendEndOfRow(cstate);
+	}
 
 	MemoryContextSwitchTo(oldcontext);
 }
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Mon_Mar__4_14_11_08_2024_631)----





^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v17] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations.
---
 src/backend/commands/copyfrom.c          |  24 +++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            |  25 +++++-
 src/include/commands/copyapi.h           | 100 +++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 155 insertions(+), 5 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
-		fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..b4a7c9c8b9 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,14 +781,22 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
-		fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
 	}
 
 	/*
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,6 +932,13 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
+	if (cstate->routine)
+	{
+		cstate->routine->CopyToOneRow(cstate, slot);
+		MemoryContextSwitchTo(oldcontext);
+		return;
+	}
+
 	if (cstate->opts.binary)
 	{
 		/* Binary per-tuple header */
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Tue_Mar__5_17_18_08_2024_468)----





^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* [PATCH v16] Add CopyFromRoutine/CopyToRountine
@ 2024-03-04 04:52  Sutou Kouhei <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Sutou Kouhei @ 2024-03-04 04:52 UTC (permalink / raw)

They are for implementing custom COPY FROM/TO format. But this is not
enough to implement custom COPY FROM/TO format yet. We'll export some
APIs to receive/send data and add "format" option to COPY FROM/TO
later.

Existing text/csv/binary format implementations don't use
CopyFromRoutine/CopyToRoutine for now. We have a patch for it but we
defer it. Because there are some mysterious profile results in spite
of we get faster runtimes. See [1] for details.

[1] https://www.postgresql.org/message-id/ZdbtQJ-p5H1_EDwE%40paquier.xyz

Note that this doesn't change existing text/csv/binary format
implementations. There are many diffs for CopyOneRowTo() but they're
caused by indentation. They don't change implementations.
---
 src/backend/commands/copyfrom.c          |  24 +++++-
 src/backend/commands/copyfromparse.c     |   5 ++
 src/backend/commands/copyto.c            | 103 ++++++++++++++---------
 src/include/commands/copyapi.h           | 100 ++++++++++++++++++++++
 src/include/commands/copyfrom_internal.h |   4 +
 src/tools/pgindent/typedefs.list         |   2 +
 6 files changed, 193 insertions(+), 45 deletions(-)
 create mode 100644 src/include/commands/copyapi.h

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index c3bc897028..9bf2f6497e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1623,12 +1623,22 @@ BeginCopyFrom(ParseState *pstate,
 
 		/* Fetch the input function and typioparam info */
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryInputInfo(att->atttypid,
 								   &in_func_oid, &typioparams[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+											&in_functions[attnum - 1],
+											&typioparams[attnum - 1]);
+
 		else
+		{
 			getTypeInputInfo(att->atttypid,
 							 &in_func_oid, &typioparams[attnum - 1]);
-		fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+			fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+		}
 
 		/* Get default info if available */
 		defexprs[attnum - 1] = NULL;
@@ -1768,10 +1778,13 @@ BeginCopyFrom(ParseState *pstate,
 		/* Read and verify binary header */
 		ReceiveCopyBinaryHeader(cstate);
 	}
-
-	/* create workspace for CopyReadAttributes results */
-	if (!cstate->opts.binary)
+	else if (cstate->routine)
 	{
+		cstate->routine->CopyFromStart(cstate, tupDesc);
+	}
+	else
+	{
+		/* create workspace for CopyReadAttributes results */
 		AttrNumber	attr_count = list_length(cstate->attnumlist);
 
 		cstate->max_fields = attr_count;
@@ -1789,6 +1802,9 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	if (cstate->routine)
+		cstate->routine->CopyFromEnd(cstate);
+
 	/* No COPY FROM related resources except memory. */
 	if (cstate->is_program)
 	{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7cacd0b752..8b15080585 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -978,6 +978,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 
 		Assert(fieldno == attr_count);
 	}
+	else if (cstate->routine)
+	{
+		if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
+			return false;
+	}
 	else
 	{
 		/* binary */
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..6080627c83 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/progress.h"
 #include "executor/execdesc.h"
 #include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
  */
 typedef struct CopyToStateData
 {
+	/* format routine */
+	const CopyToRoutine *routine;
+
 	/* low-level state data */
 	CopyDest	copy_dest;		/* type of copy source/destination */
 	FILE	   *copy_file;		/* used if copy_dest == COPY_FILE */
@@ -777,14 +781,22 @@ DoCopyTo(CopyToState cstate)
 		Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
 
 		if (cstate->opts.binary)
+		{
 			getTypeBinaryOutputInfo(attr->atttypid,
 									&out_func_oid,
 									&isvarlena);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
+		else if (cstate->routine)
+			cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+										   &cstate->out_functions[attnum - 1]);
 		else
+		{
 			getTypeOutputInfo(attr->atttypid,
 							  &out_func_oid,
 							  &isvarlena);
-		fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+			fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+		}
 	}
 
 	/*
@@ -811,6 +823,8 @@ DoCopyTo(CopyToState cstate)
 		tmp = 0;
 		CopySendInt32(cstate, tmp);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToStart(cstate, tupDesc);
 	else
 	{
 		/*
@@ -892,6 +906,8 @@ DoCopyTo(CopyToState cstate)
 		/* Need to flush out the trailer */
 		CopySendEndOfRow(cstate);
 	}
+	else if (cstate->routine)
+		cstate->routine->CopyToEnd(cstate);
 
 	MemoryContextDelete(cstate->rowcontext);
 
@@ -916,61 +932,66 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
 	MemoryContextReset(cstate->rowcontext);
 	oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
 
-	if (cstate->opts.binary)
+	if (cstate->routine)
+		cstate->routine->CopyToOneRow(cstate, slot);
+	else
 	{
-		/* Binary per-tuple header */
-		CopySendInt16(cstate, list_length(cstate->attnumlist));
-	}
-
-	/* Make sure the tuple is fully deconstructed */
-	slot_getallattrs(slot);
-
-	foreach(cur, cstate->attnumlist)
-	{
-		int			attnum = lfirst_int(cur);
-		Datum		value = slot->tts_values[attnum - 1];
-		bool		isnull = slot->tts_isnull[attnum - 1];
-
-		if (!cstate->opts.binary)
+		if (cstate->opts.binary)
 		{
-			if (need_delim)
-				CopySendChar(cstate, cstate->opts.delim[0]);
-			need_delim = true;
+			/* Binary per-tuple header */
+			CopySendInt16(cstate, list_length(cstate->attnumlist));
 		}
 
-		if (isnull)
-		{
-			if (!cstate->opts.binary)
-				CopySendString(cstate, cstate->opts.null_print_client);
-			else
-				CopySendInt32(cstate, -1);
-		}
-		else
+		/* Make sure the tuple is fully deconstructed */
+		slot_getallattrs(slot);
+
+		foreach(cur, cstate->attnumlist)
 		{
+			int			attnum = lfirst_int(cur);
+			Datum		value = slot->tts_values[attnum - 1];
+			bool		isnull = slot->tts_isnull[attnum - 1];
+
 			if (!cstate->opts.binary)
 			{
-				string = OutputFunctionCall(&out_functions[attnum - 1],
-											value);
-				if (cstate->opts.csv_mode)
-					CopyAttributeOutCSV(cstate, string,
-										cstate->opts.force_quote_flags[attnum - 1]);
+				if (need_delim)
+					CopySendChar(cstate, cstate->opts.delim[0]);
+				need_delim = true;
+			}
+
+			if (isnull)
+			{
+				if (!cstate->opts.binary)
+					CopySendString(cstate, cstate->opts.null_print_client);
 				else
-					CopyAttributeOutText(cstate, string);
+					CopySendInt32(cstate, -1);
 			}
 			else
 			{
-				bytea	   *outputbytes;
+				if (!cstate->opts.binary)
+				{
+					string = OutputFunctionCall(&out_functions[attnum - 1],
+												value);
+					if (cstate->opts.csv_mode)
+						CopyAttributeOutCSV(cstate, string,
+											cstate->opts.force_quote_flags[attnum - 1]);
+					else
+						CopyAttributeOutText(cstate, string);
+				}
+				else
+				{
+					bytea	   *outputbytes;
 
-				outputbytes = SendFunctionCall(&out_functions[attnum - 1],
-											   value);
-				CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
-				CopySendData(cstate, VARDATA(outputbytes),
-							 VARSIZE(outputbytes) - VARHDRSZ);
+					outputbytes = SendFunctionCall(&out_functions[attnum - 1],
+												   value);
+					CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
+					CopySendData(cstate, VARDATA(outputbytes),
+								 VARSIZE(outputbytes) - VARHDRSZ);
+				}
 			}
 		}
-	}
 
-	CopySendEndOfRow(cstate);
+		CopySendEndOfRow(cstate);
+	}
 
 	MemoryContextSwitchTo(oldcontext);
 }
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..635c4cbff2
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ *	  API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation.  Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+	/*
+	 * Called when COPY FROM is started to set up the input functions
+	 * associated to the relation's attributes writing to.  `finfo` can be
+	 * optionally filled to provide the catalog information of the input
+	 * function.  `typioparam` can be optionally filled to define the OID of
+	 * the type to pass to the input function.  `atttypid` is the OID of data
+	 * type used by the relation's attribute.
+	 */
+	void		(*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+								   FmgrInfo *finfo, Oid *typioparam);
+
+	/*
+	 * Called when COPY FROM is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * to be copied.  This can be used for any initialization steps required
+	 * by a format.
+	 */
+	void		(*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+	 *
+	 * 'econtext' is used to evaluate default expression for each column that
+	 * is either not read from the file or is using the DEFAULT option of COPY
+	 * FROM.  It is NULL if no default values are used.
+	 *
+	 * Returns false if there are no more tuples to copy.
+	 */
+	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls);
+
+	/* Called when COPY FROM has ended. */
+	void		(*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation.   Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+	/*
+	 * Called when COPY TO is started to set up the output functions
+	 * associated to the relation's attributes reading from.  `finfo` can be
+	 * optionally filled.  `atttypid` is the OID of data type used by the
+	 * relation's attribute.
+	 */
+	void		(*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+								  FmgrInfo *finfo);
+
+	/*
+	 * Called when COPY TO is started.
+	 *
+	 * `tupDesc` is the tuple descriptor of the relation from where the data
+	 * is read.
+	 */
+	void		(*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+	/*
+	 * Copy one row for COPY TO.
+	 *
+	 * `slot` is the tuple slot where the data is emitted.
+	 */
+	void		(*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+	/* Called when COPY TO has ended */
+	void		(*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif							/* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..509b9e92a1 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
 #define COPYFROM_INTERNAL_H
 
 #include "commands/copy.h"
+#include "commands/copyapi.h"
 #include "commands/trigger.h"
 #include "nodes/miscnodes.h"
 
@@ -58,6 +59,9 @@ typedef enum CopyInsertMethod
  */
 typedef struct CopyFromStateData
 {
+	/* format routine */
+	const CopyFromRoutine *routine;
+
 	/* low-level state data */
 	CopySource	copy_src;		/* type of copy source */
 	FILE	   *copy_file;		/* used if copy_src == COPY_FILE */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..a5ae161ca5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConvertRowtypeExpr
 CookedConstraint
 CopyDest
 CopyFormatOptions
+CopyFromRoutine
 CopyFromState
 CopyFromStateData
 CopyHeaderChoice
@@ -484,6 +485,7 @@ CopyMultiInsertInfo
 CopyOnErrorChoice
 CopySource
 CopyStmt
+CopyToRoutine
 CopyToState
 CopyToStateData
 Cost
-- 
2.43.0


----Next_Part(Mon_Mar__4_14_11_08_2024_631)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v16-0001-Add-CopyFromRoutine-CopyToRountine-w.patch"



^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* Re: tests fail on windows with default git settings
@ 2024-07-09 16:32  Andres Freund <[email protected]>
  0 siblings, 1 reply; 21+ messages in thread

From: Andres Freund @ 2024-07-09 16:32 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

Hi,

On 2024-07-09 14:52:39 +0100, Dave Page wrote:
> I have 4 different diff.exe's on my ~6 week old build VM (not counting
> shims), all of which seem to support --strip-trailing-cr. Those builds came
> with:
>
> - git
> - VC++
> - diffutils (installed by chocolatey)
> - vcpkg
>
> I think it's reasonable to assume it'll be supported.

I think the more likely issue would be an older setup with an older diff,
people on windows seem to not want to touch a working setup ever :). But we
can deal with that if reports about it come in.


> > > Not sure what the issue is with pg_bsd_indent, though.
> >
>
> Yeah - that's odd, as that test always passes for me, with or without
> autocrlf.

Huh.


> The other failures I see are the following, which I'm just starting to dig
> into:
>
>  26/298 postgresql:recovery / recovery/019_replslot_limit
>             ERROR            43.05s   exit status 2
>  44/298 postgresql:recovery / recovery/027_stream_regress
>             ERROR           383.08s   exit status 1
>  50/298 postgresql:recovery / recovery/035_standby_logical_decoding
>             ERROR           138.06s   exit status 25
>  68/298 postgresql:recovery / recovery/040_standby_failover_slots_sync
>              ERROR           132.87s   exit status 25
> 170/298 postgresql:pg_dump / pg_dump/002_pg_dump
>              ERROR            93.45s   exit status 2
> 233/298 postgresql:bloom / bloom/001_wal
>              ERROR            54.47s   exit status 2
> 236/298 postgresql:subscription / subscription/001_rep_changes
>              ERROR            46.46s   exit status 2
> 246/298 postgresql:subscription / subscription/010_truncate
>             ERROR            47.69s   exit status 2
> 253/298 postgresql:subscription / subscription/013_partition
>              ERROR           125.63s   exit status 25
> 255/298 postgresql:subscription / subscription/022_twophase_cascade
>             ERROR            58.13s   exit status 2
> 257/298 postgresql:subscription / subscription/015_stream
>             ERROR           128.32s   exit status 2
> 262/298 postgresql:subscription / subscription/028_row_filter
>             ERROR            43.14s   exit status 2
> 263/298 postgresql:subscription / subscription/027_nosuperuser
>              ERROR           102.02s   exit status 2
> 269/298 postgresql:subscription / subscription/031_column_list
>              ERROR           123.16s   exit status 2
> 271/298 postgresql:subscription / subscription/032_subscribe_use_index
>              ERROR           139.33s   exit status 2

Hm, it'd be good to see some of errors behind that ([1]).

I suspect it might be related to conflicting ports. I had to use
PG_TEST_USE_UNIX_SOCKETS to avoid random tests from failing:

          # use unix socket to prevent port conflicts
          $env:PG_TEST_USE_UNIX_SOCKETS = 1;
          # otherwise pg_regress insists on creating the directory and does it
          # in a non-existing place, this needs to be fixed :(
          mkdir d:/sockets
          $env:PG_REGRESS_SOCK_DIR = "d:/sockets/"


FWIW, building a tree with the patches I sent to the list last night and
changes to make postgresql-dev.yml use a git checkout, I get:

https://github.com/anarazel/winpgbuild/actions/runs/9852370209/job/27200784987#step:12:469

Ok:                 281
Expected Fail:      0
Fail:               0
Unexpected Pass:    0
Skipped:            17
Timeout:            0

This is without readline and pltcl, as neither is currently built as part of
winpgbuild. Otherwise it has all applicable dependencies enabled (no bonjour,
bsd_auth, dtrace, llvm, pam, selinux, systemd, but that's afaict expected).

Greetings,

Andres Freund


[1] I plan to submit a PR that'll collect the necessary information






^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* Re: tests fail on windows with default git settings
@ 2024-07-10 09:30  Dave Page <[email protected]>
  parent: Andres Freund <[email protected]>
  0 siblings, 1 reply; 21+ messages in thread

From: Dave Page @ 2024-07-10 09:30 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

On Tue, 9 Jul 2024 at 17:32, Andres Freund <[email protected]> wrote:

> Hi,
>
> On 2024-07-09 14:52:39 +0100, Dave Page wrote:
> > I have 4 different diff.exe's on my ~6 week old build VM (not counting
> > shims), all of which seem to support --strip-trailing-cr. Those builds
> came
> > with:
> >
> > - git
> > - VC++
> > - diffutils (installed by chocolatey)
> > - vcpkg
> >
> > I think it's reasonable to assume it'll be supported.
>
> I think the more likely issue would be an older setup with an older diff,
> people on windows seem to not want to touch a working setup ever :). But we
> can deal with that if reports about it come in.
>

They've got to move to meson/ninja anyway, so... <shrug>.


>
>
> > > > Not sure what the issue is with pg_bsd_indent, though.
> > >
> >
> > Yeah - that's odd, as that test always passes for me, with or without
> > autocrlf.
>
> Huh.
>
>
> > The other failures I see are the following, which I'm just starting to
> dig
> > into:
> >
> >  26/298 postgresql:recovery / recovery/019_replslot_limit
> >             ERROR            43.05s   exit status 2
> >  44/298 postgresql:recovery / recovery/027_stream_regress
> >             ERROR           383.08s   exit status 1
> >  50/298 postgresql:recovery / recovery/035_standby_logical_decoding
> >             ERROR           138.06s   exit status 25
> >  68/298 postgresql:recovery / recovery/040_standby_failover_slots_sync
> >              ERROR           132.87s   exit status 25
> > 170/298 postgresql:pg_dump / pg_dump/002_pg_dump
> >              ERROR            93.45s   exit status 2
> > 233/298 postgresql:bloom / bloom/001_wal
> >              ERROR            54.47s   exit status 2
> > 236/298 postgresql:subscription / subscription/001_rep_changes
> >              ERROR            46.46s   exit status 2
> > 246/298 postgresql:subscription / subscription/010_truncate
> >             ERROR            47.69s   exit status 2
> > 253/298 postgresql:subscription / subscription/013_partition
> >              ERROR           125.63s   exit status 25
> > 255/298 postgresql:subscription / subscription/022_twophase_cascade
> >             ERROR            58.13s   exit status 2
> > 257/298 postgresql:subscription / subscription/015_stream
> >             ERROR           128.32s   exit status 2
> > 262/298 postgresql:subscription / subscription/028_row_filter
> >             ERROR            43.14s   exit status 2
> > 263/298 postgresql:subscription / subscription/027_nosuperuser
> >              ERROR           102.02s   exit status 2
> > 269/298 postgresql:subscription / subscription/031_column_list
> >              ERROR           123.16s   exit status 2
> > 271/298 postgresql:subscription / subscription/032_subscribe_use_index
> >              ERROR           139.33s   exit status 2
>
> Hm, it'd be good to see some of errors behind that ([1]).
>
> I suspect it might be related to conflicting ports. I had to use
> PG_TEST_USE_UNIX_SOCKETS to avoid random tests from failing:
>
>           # use unix socket to prevent port conflicts
>           $env:PG_TEST_USE_UNIX_SOCKETS = 1;
>           # otherwise pg_regress insists on creating the directory and
> does it
>           # in a non-existing place, this needs to be fixed :(
>           mkdir d:/sockets
>           $env:PG_REGRESS_SOCK_DIR = "d:/sockets/"
>

No, it all seems to be fallout from GSSAPI being included in the build. If
I build without that, everything passes. Most of the tests are failing with
a "too many clients already" error, but a handful do seem to include auth
related errors as well. For example, this is from




>
>
> FWIW, building a tree with the patches I sent to the list last night and
> changes to make postgresql-dev.yml use a git checkout, I get:
>
>
> https://github.com/anarazel/winpgbuild/actions/runs/9852370209/job/27200784987#step:12:469
>
> Ok:                 281
> Expected Fail:      0
> Fail:               0
> Unexpected Pass:    0
> Skipped:            17
> Timeout:            0
>
> This is without readline and pltcl, as neither is currently built as part
> of
> winpgbuild. Otherwise it has all applicable dependencies enabled (no
> bonjour,
> bsd_auth, dtrace, llvm, pam, selinux, systemd, but that's afaict expected).
>
> Greetings,
>
> Andres Freund
>
>
> [1] I plan to submit a PR that'll collect the necessary information
>


-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com


^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* Re: tests fail on windows with default git settings
@ 2024-07-10 09:35  Dave Page <[email protected]>
  parent: Dave Page <[email protected]>
  0 siblings, 1 reply; 21+ messages in thread

From: Dave Page @ 2024-07-10 09:35 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

Sorry - somehow managed to send whilst pasting in logs...

On Wed, 10 Jul 2024 at 10:30, Dave Page <[email protected]> wrote:

>
>
> On Tue, 9 Jul 2024 at 17:32, Andres Freund <[email protected]> wrote:
>
>> Hi,
>>
>> On 2024-07-09 14:52:39 +0100, Dave Page wrote:
>> > I have 4 different diff.exe's on my ~6 week old build VM (not counting
>> > shims), all of which seem to support --strip-trailing-cr. Those builds
>> came
>> > with:
>> >
>> > - git
>> > - VC++
>> > - diffutils (installed by chocolatey)
>> > - vcpkg
>> >
>> > I think it's reasonable to assume it'll be supported.
>>
>> I think the more likely issue would be an older setup with an older diff,
>> people on windows seem to not want to touch a working setup ever :). But
>> we
>> can deal with that if reports about it come in.
>>
>
> They've got to move to meson/ninja anyway, so... <shrug>.
>
>
>>
>>
>> > > > Not sure what the issue is with pg_bsd_indent, though.
>> > >
>> >
>> > Yeah - that's odd, as that test always passes for me, with or without
>> > autocrlf.
>>
>> Huh.
>>
>>
>> > The other failures I see are the following, which I'm just starting to
>> dig
>> > into:
>> >
>> >  26/298 postgresql:recovery / recovery/019_replslot_limit
>> >             ERROR            43.05s   exit status 2
>> >  44/298 postgresql:recovery / recovery/027_stream_regress
>> >             ERROR           383.08s   exit status 1
>> >  50/298 postgresql:recovery / recovery/035_standby_logical_decoding
>> >             ERROR           138.06s   exit status 25
>> >  68/298 postgresql:recovery / recovery/040_standby_failover_slots_sync
>> >              ERROR           132.87s   exit status 25
>> > 170/298 postgresql:pg_dump / pg_dump/002_pg_dump
>> >              ERROR            93.45s   exit status 2
>> > 233/298 postgresql:bloom / bloom/001_wal
>> >              ERROR            54.47s   exit status 2
>> > 236/298 postgresql:subscription / subscription/001_rep_changes
>> >              ERROR            46.46s   exit status 2
>> > 246/298 postgresql:subscription / subscription/010_truncate
>> >             ERROR            47.69s   exit status 2
>> > 253/298 postgresql:subscription / subscription/013_partition
>> >              ERROR           125.63s   exit status 25
>> > 255/298 postgresql:subscription / subscription/022_twophase_cascade
>> >             ERROR            58.13s   exit status 2
>> > 257/298 postgresql:subscription / subscription/015_stream
>> >             ERROR           128.32s   exit status 2
>> > 262/298 postgresql:subscription / subscription/028_row_filter
>> >             ERROR            43.14s   exit status 2
>> > 263/298 postgresql:subscription / subscription/027_nosuperuser
>> >              ERROR           102.02s   exit status 2
>> > 269/298 postgresql:subscription / subscription/031_column_list
>> >              ERROR           123.16s   exit status 2
>> > 271/298 postgresql:subscription / subscription/032_subscribe_use_index
>> >              ERROR           139.33s   exit status 2
>>
>> Hm, it'd be good to see some of errors behind that ([1]).
>>
>> I suspect it might be related to conflicting ports. I had to use
>> PG_TEST_USE_UNIX_SOCKETS to avoid random tests from failing:
>>
>>           # use unix socket to prevent port conflicts
>>           $env:PG_TEST_USE_UNIX_SOCKETS = 1;
>>           # otherwise pg_regress insists on creating the directory and
>> does it
>>           # in a non-existing place, this needs to be fixed :(
>>           mkdir d:/sockets
>>           $env:PG_REGRESS_SOCK_DIR = "d:/sockets/"
>>
>
> No, it all seems to be fallout from GSSAPI being included in the build. If
> I build without that, everything passes. Most of the tests are failing with
> a "too many clients already" error, but a handful do seem to include GSSAPI
> auth related errors as well. For example, this is from
>


... this is from subscription/001_rep_changes:

[14:46:57.723](2.318s) ok 11 - check rows on subscriber after table drop
from publication
connection error: 'psql: error: connection to server at "127.0.0.1", port
58059 failed: could not initiate GSSAPI security context: No credentials
were supplied, or the credentials were unavailable or inaccessible:
Credential cache is empty
connection to server at "127.0.0.1", port 58059 failed: FATAL:  sorry, too
many clients already'
while running 'psql -XAtq -d port=58059 host=127.0.0.1 dbname='postgres' -f
- -v ON_ERROR_STOP=1' at
C:/Users/dpage/git/postgresql/src/test/perl/PostgreSQL/Test/Cluster.pm line
2129.
# Postmaster PID for node "publisher" is 14488
### Stopping node "publisher" using mode immediate
# Running: pg_ctl -D
C:\Users\dpage\git\postgresql\build/testrun/subscription/001_rep_changes\data/t_001_rep_changes_publisher_data/pgdata
-m immediate stop
waiting for server to shut down.... done
server stopped
# No postmaster PID for node "publisher"
# Postmaster PID for node "subscriber" is 15012
### Stopping node "subscriber" using mode immediate
# Running: pg_ctl -D
C:\Users\dpage\git\postgresql\build/testrun/subscription/001_rep_changes\data/t_001_rep_changes_subscriber_data/pgdata
-m immediate stop
waiting for server to shut down.... done
server stopped
# No postmaster PID for node "subscriber"
[14:46:59.068](1.346s) # Tests were run but no plan was declared and
done_testing() was not seen.
[14:46:59.069](0.000s) # Looks like your test exited with 2 just after 11.


>
>
>
>
>>
>>
>> FWIW, building a tree with the patches I sent to the list last night and
>> changes to make postgresql-dev.yml use a git checkout, I get:
>>
>>
>> https://github.com/anarazel/winpgbuild/actions/runs/9852370209/job/27200784987#step:12:469
>>
>> Ok:                 281
>> Expected Fail:      0
>> Fail:               0
>> Unexpected Pass:    0
>> Skipped:            17
>> Timeout:            0
>>
>> This is without readline and pltcl, as neither is currently built as part
>> of
>> winpgbuild. Otherwise it has all applicable dependencies enabled (no
>> bonjour,
>> bsd_auth, dtrace, llvm, pam, selinux, systemd, but that's afaict
>> expected).
>>
>> Greetings,
>>
>> Andres Freund
>>
>>
>> [1] I plan to submit a PR that'll collect the necessary information
>>
>
>
> --
> Dave Page
> pgAdmin: https://www.pgadmin.org
> PostgreSQL: https://www.postgresql.org
> EDB: https://www.enterprisedb.com
>
>

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com


^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* Re: tests fail on windows with default git settings
@ 2024-07-11 15:49  Dave Page <[email protected]>
  parent: Dave Page <[email protected]>
  0 siblings, 1 reply; 21+ messages in thread

From: Dave Page @ 2024-07-11 15:49 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>

On Wed, 10 Jul 2024 at 10:35, Dave Page <[email protected]> wrote:

> > The other failures I see are the following, which I'm just starting to
>>> dig
>>> > into:
>>> >
>>> >  26/298 postgresql:recovery / recovery/019_replslot_limit
>>> >             ERROR            43.05s   exit status 2
>>> >  44/298 postgresql:recovery / recovery/027_stream_regress
>>> >             ERROR           383.08s   exit status 1
>>> >  50/298 postgresql:recovery / recovery/035_standby_logical_decoding
>>> >             ERROR           138.06s   exit status 25
>>> >  68/298 postgresql:recovery / recovery/040_standby_failover_slots_sync
>>> >              ERROR           132.87s   exit status 25
>>> > 170/298 postgresql:pg_dump / pg_dump/002_pg_dump
>>> >              ERROR            93.45s   exit status 2
>>> > 233/298 postgresql:bloom / bloom/001_wal
>>> >              ERROR            54.47s   exit status 2
>>> > 236/298 postgresql:subscription / subscription/001_rep_changes
>>> >              ERROR            46.46s   exit status 2
>>> > 246/298 postgresql:subscription / subscription/010_truncate
>>> >             ERROR            47.69s   exit status 2
>>> > 253/298 postgresql:subscription / subscription/013_partition
>>> >              ERROR           125.63s   exit status 25
>>> > 255/298 postgresql:subscription / subscription/022_twophase_cascade
>>> >             ERROR            58.13s   exit status 2
>>> > 257/298 postgresql:subscription / subscription/015_stream
>>> >             ERROR           128.32s   exit status 2
>>> > 262/298 postgresql:subscription / subscription/028_row_filter
>>> >             ERROR            43.14s   exit status 2
>>> > 263/298 postgresql:subscription / subscription/027_nosuperuser
>>> >              ERROR           102.02s   exit status 2
>>> > 269/298 postgresql:subscription / subscription/031_column_list
>>> >              ERROR           123.16s   exit status 2
>>> > 271/298 postgresql:subscription / subscription/032_subscribe_use_index
>>> >              ERROR           139.33s   exit status 2
>>>
>>> Hm, it'd be good to see some of errors behind that ([1]).
>>>
>>> I suspect it might be related to conflicting ports. I had to use
>>> PG_TEST_USE_UNIX_SOCKETS to avoid random tests from failing:
>>>
>>>           # use unix socket to prevent port conflicts
>>>           $env:PG_TEST_USE_UNIX_SOCKETS = 1;
>>>           # otherwise pg_regress insists on creating the directory and
>>> does it
>>>           # in a non-existing place, this needs to be fixed :(
>>>           mkdir d:/sockets
>>>           $env:PG_REGRESS_SOCK_DIR = "d:/sockets/"
>>>
>>
>> No, it all seems to be fallout from GSSAPI being included in the build.
>> If I build without that, everything passes. Most of the tests are failing
>> with a "too many clients already" error, but a handful do seem to include
>> GSSAPI auth related errors as well. For example, this is from
>>
>
>
> ... this is from subscription/001_rep_changes:
>
> [14:46:57.723](2.318s) ok 11 - check rows on subscriber after table drop
> from publication
> connection error: 'psql: error: connection to server at "127.0.0.1", port
> 58059 failed: could not initiate GSSAPI security context: No credentials
> were supplied, or the credentials were unavailable or inaccessible:
> Credential cache is empty
> connection to server at "127.0.0.1", port 58059 failed: FATAL:  sorry, too
> many clients already'
> while running 'psql -XAtq -d port=58059 host=127.0.0.1 dbname='postgres'
> -f - -v ON_ERROR_STOP=1' at
> C:/Users/dpage/git/postgresql/src/test/perl/PostgreSQL/Test/Cluster.pm line
> 2129.
> # Postmaster PID for node "publisher" is 14488
> ### Stopping node "publisher" using mode immediate
> # Running: pg_ctl -D
> C:\Users\dpage\git\postgresql\build/testrun/subscription/001_rep_changes\data/t_001_rep_changes_publisher_data/pgdata
> -m immediate stop
> waiting for server to shut down.... done
> server stopped
> # No postmaster PID for node "publisher"
> # Postmaster PID for node "subscriber" is 15012
> ### Stopping node "subscriber" using mode immediate
> # Running: pg_ctl -D
> C:\Users\dpage\git\postgresql\build/testrun/subscription/001_rep_changes\data/t_001_rep_changes_subscriber_data/pgdata
> -m immediate stop
> waiting for server to shut down.... done
> server stopped
> # No postmaster PID for node "subscriber"
> [14:46:59.068](1.346s) # Tests were run but no plan was declared and
> done_testing() was not seen.
> [14:46:59.069](0.000s) # Looks like your test exited with 2 just after 11.
>

So I received an off-list tip to checkout [1], a discussion around GSSAPI
causing test failures on windows that Alexander Lakhin was looking at.
Thomas Munro's v2 patch to try to address the issue brought me down to just
a single test failure with GSSAPI enabled on 17b2 (with a second, simple
fix for the OpenSSL/Kerberos/x509 issue): pg_dump/002_pg_dump. The
relevant section from the log looks like this:

[15:28:42.692](0.006s) not ok 2 - connecting to a non-existent database:
matches
[15:28:42.693](0.001s) #   Failed test 'connecting to a non-existent
database: matches'
#   at C:/Users/dpage/git/postgresql/src/bin/pg_dump/t/002_pg_dump.pl line
4689.
[15:28:42.694](0.001s) #                   'pg_dump: error: connection to
server at "127.0.0.1", port 53834 failed: could not initiate GSSAPI
security context: No credentials were supplied, or the credentials were
unavailable or inaccessible: Credential cache is empty
# connection to server at "127.0.0.1", port 53834 failed: FATAL:  database
"qqq" does not exist
# '
#     doesn't match '(?^:pg_dump: error: connection to server .* failed:
FATAL:  database "qqq" does not exist)'
# Running: pg_dump -d regression_invalid

We could tweak the regex I suppose, but that just seems like it's skirting
around the actual problem. I could also get on board with Tom's idea of
deprecating GSSAPI for Windows, assuming that SSPI can handle everything
users might reasonably need (I really have no idea how likely that is).

[1] ttps://www.postgresql.org/message-id/[email protected]
<https://www.postgresql.org/message-id/[email protected];

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com


^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* Re: tests fail on windows with default git settings
@ 2024-07-13 22:00  Thomas Munro <[email protected]>
  parent: Dave Page <[email protected]>
  0 siblings, 2 replies; 21+ messages in thread

From: Thomas Munro @ 2024-07-13 22:00 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

On Fri, Jul 12, 2024 at 3:49 AM Dave Page <[email protected]> wrote:
> So I received an off-list tip to checkout [1], a discussion around GSSAPI causing test failures on windows that Alexander Lakhin was looking at. Thomas Munro's v2 patch to try to address the issue brought me down to just a single test failure with GSSAPI enabled on 17b2 (with a second, simple fix for the OpenSSL/Kerberos/x509 issue): pg_dump/002_pg_dump. The relevant section from the log looks like this:

I pushed that (ba9fcac7).

> [15:28:42.692](0.006s) not ok 2 - connecting to a non-existent database: matches
> [15:28:42.693](0.001s) #   Failed test 'connecting to a non-existent database: matches'
> #   at C:/Users/dpage/git/postgresql/src/bin/pg_dump/t/002_pg_dump.pl line 4689.
> [15:28:42.694](0.001s) #                   'pg_dump: error: connection to server at "127.0.0.1", port 53834 failed: could not initiate GSSAPI security context: No credentials were supplied, or the credentials were unavailable or inaccessible: Credential cache is empty
> # connection to server at "127.0.0.1", port 53834 failed: FATAL:  database "qqq" does not exist
> # '
> #     doesn't match '(?^:pg_dump: error: connection to server .* failed: FATAL:  database "qqq" does not exist)'

Does it help if you revert 29992a6?






^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* Re: tests fail on windows with default git settings
@ 2024-07-16 00:22  Thomas Munro <[email protected]>
  parent: Thomas Munro <[email protected]>
  1 sibling, 0 replies; 21+ messages in thread

From: Thomas Munro @ 2024-07-16 00:22 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

On Sun, Jul 14, 2024 at 10:00 AM Thomas Munro <[email protected]> wrote:
> On Fri, Jul 12, 2024 at 3:49 AM Dave Page <[email protected]> wrote:
> > #     doesn't match '(?^:pg_dump: error: connection to server .* failed: FATAL:  database "qqq" does not exist)'
>
> Does it help if you revert 29992a6?

FWIW I just happened to notice the same failure on Cirrus, in the
github.com/postgres/postgres master branch:

https://cirrus-ci.com/task/5382396705505280

Your failure mentions GSSAPI and the above doesn't, but that'd be
because for Cirrus CI we have PG_TEST_USE_UNIX_SOCKETS so it's using
AF_UNIX.  At one point I proposed deleting that weird GSAPPI stuff and
using AF_UNIX always on Windows[1], but the feedback was that I should
instead teach the whole test suite to be able to use AF_UNIX or
AF_INET* on all OSes and I never got back to it.

The error does seem be the never-ending saga from this and other threads:

https://www.postgresql.org/message-id/flat/90b34057-4176-7bb0-0dbb-9822a5f6425b%40greiz-reinsdorf.de

My uninformed impression is that graceful socket shutdowns would very
likely fix the class of lost-final-message problem where the client
does recv() next, including this case IIUC.  It's only a partial
improvement though: if the client calls send() next, I think it can
still drop buffered received data, so this graceful shutdown stuff
doesn't quite get us to the same situation as Unix all points in the
protocol.  The real world case where that second case comes up is
where the client sends a new query and on Unix gets a buffered error
message saying the backend has exited due to idle timeout, but on
Windows gets a connection reset message.  I've wondered before if you
could fix (or narrow to almost zero?) that by giving libpq a mode
where it calls poll() to check for buffered readable data every single
time it's about to send.

[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGK30uLx9dpgkYwomgH0WVLUHytkChDgf3iUM2zp0pf_nA%40m...






^ permalink  raw  reply  [nested|flat] 21+ messages in thread

* Re: tests fail on windows with default git settings
@ 2024-07-24 15:49  Dave Page <[email protected]>
  parent: Thomas Munro <[email protected]>
  1 sibling, 0 replies; 21+ messages in thread

From: Dave Page @ 2024-07-24 15:49 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

Hi

On Sat, 13 Jul 2024 at 23:01, Thomas Munro <[email protected]> wrote:

> On Fri, Jul 12, 2024 at 3:49 AM Dave Page <[email protected]> wrote:
> > So I received an off-list tip to checkout [1], a discussion around
> GSSAPI causing test failures on windows that Alexander Lakhin was looking
> at. Thomas Munro's v2 patch to try to address the issue brought me down to
> just a single test failure with GSSAPI enabled on 17b2 (with a second,
> simple fix for the OpenSSL/Kerberos/x509 issue): pg_dump/002_pg_dump. The
> relevant section from the log looks like this:
>
> I pushed that (ba9fcac7).
>
> > [15:28:42.692](0.006s) not ok 2 - connecting to a non-existent database:
> matches
> > [15:28:42.693](0.001s) #   Failed test 'connecting to a non-existent
> database: matches'
> > #   at C:/Users/dpage/git/postgresql/src/bin/pg_dump/t/002_pg_dump.pl
> line 4689.
> > [15:28:42.694](0.001s) #                   'pg_dump: error: connection
> to server at "127.0.0.1", port 53834 failed: could not initiate GSSAPI
> security context: No credentials were supplied, or the credentials were
> unavailable or inaccessible: Credential cache is empty
> > # connection to server at "127.0.0.1", port 53834 failed: FATAL:
> database "qqq" does not exist
> > # '
> > #     doesn't match '(?^:pg_dump: error: connection to server .* failed:
> FATAL:  database "qqq" does not exist)'
>
> Does it help if you revert 29992a6?
>

Sorry for the delay - things got crazy busy for a while.

No, reverting that commit does not help.

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

PGDay UK 2024, 11th September, London: https://2024.pgday.uk/


^ permalink  raw  reply  [nested|flat] 21+ messages in thread

end of thread, other threads:[~2024-07-24 15:49 UTC | newest]

Thread overview: 21+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-10-11 01:03 [PATCH 2/8] Write WAL for empty nbtree index build Kyotaro Horiguchi <[email protected]>
2018-10-11 01:03 [PATCH 2/7] Write WAL for empty nbtree index build Kyotaro Horiguchi <[email protected]>
2018-10-11 01:03 [PATCH 2/7] Write WAL for empty nbtree index build Kyotaro Horiguchi <[email protected]>
2018-10-11 01:03 [PATCH 2/7] Write WAL for empty nbtree index build Kyotaro Horiguchi <[email protected]>
2018-10-11 01:03 [PATCH 2/4] Write WAL for empty nbtree index build Kyotaro Horiguchi <[email protected]>
2024-03-04 04:52 [PATCH v16] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-03-04 04:52 [PATCH v17] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-03-04 04:52 [PATCH v16] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-03-04 04:52 [PATCH v17] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-03-04 04:52 [PATCH v16] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-03-04 04:52 [PATCH v16] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-03-04 04:52 [PATCH v17] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-03-04 04:52 [PATCH v16] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-03-04 04:52 [PATCH v16] Add CopyFromRoutine/CopyToRountine Sutou Kouhei <[email protected]>
2024-07-09 16:32 Re: tests fail on windows with default git settings Andres Freund <[email protected]>
2024-07-10 09:30 ` Re: tests fail on windows with default git settings Dave Page <[email protected]>
2024-07-10 09:35   ` Re: tests fail on windows with default git settings Dave Page <[email protected]>
2024-07-11 15:49     ` Re: tests fail on windows with default git settings Dave Page <[email protected]>
2024-07-13 22:00       ` Re: tests fail on windows with default git settings Thomas Munro <[email protected]>
2024-07-16 00:22         ` Re: tests fail on windows with default git settings Thomas Munro <[email protected]>
2024-07-24 15:49         ` Re: tests fail on windows with default git settings Dave Page <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox