v24-0004-fixup-minor-updates-for-COPY-FROM-refactoring.patch

From b6b5c0409eed0558320e39bd642b2be17f17f590 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 19 Nov 2024 13:46:06 -0800
Subject: [PATCH v24 4/4] fixup: minor updates for COPY FROM refactoring.

includes:

- cleanup comments.
- reorder function definitions.
---
 src/backend/commands/copyfrom.c          | 161 +++++++++++------------
 src/backend/commands/copyfromparse.c     |  78 +++++------
 src/include/commands/copyapi.h           |  26 ++--
 src/include/commands/copyfrom_internal.h |   2 +-
 4 files changed, 121 insertions(+), 146 deletions(-)

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index e77986f9a9..7f1de8a42b 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -106,31 +106,65 @@ typedef struct CopyMultiInsertInfo
 /* non-export function prototypes */
 static void ClosePipeFromProgram(CopyFromState cstate);
 
-
 /*
- * CopyFromRoutine implementations for text and CSV.
+ * built-in format-specific routines. One-row callbacks are defined in
+ * copyfromparse.c
  */
+static void CopyFromTextLikeInFunc(CopyFromState cstate, Oid atttypid, FmgrInfo *finfo,
+								   Oid *typioparam);
+static void CopyFromTextLikeStart(CopyFromState cstate, TupleDesc tupDesc);
+static void CopyFromTextLikeEnd(CopyFromState cstate);
+static void CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid,
+								 FmgrInfo *finfo, Oid *typioparam);
+static void CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc);
+static void CopyFromBinaryEnd(CopyFromState cstate);
+
 
 /*
- * CopyFromTextLikeInFunc
- *
- * Assign input function data for a relation's attribute in text/CSV format.
+ * COPY FROM routines for built-in formats.
++
+ * CSV and text formats share the same TextLike routines except for the
+ * one-row callback.
  */
-static void
-CopyFromTextLikeInFunc(CopyFromState cstate, Oid atttypid,
-					   FmgrInfo *finfo, Oid *typioparam)
+
+/* TEXT format */
+static const CopyFromRoutine CopyFromRoutineText = {
+	.CopyFromInFunc = CopyFromTextLikeInFunc,
+	.CopyFromStart = CopyFromTextLikeStart,
+	.CopyFromOneRow = CopyFromTextOneRow,
+	.CopyFromEnd = CopyFromTextLikeEnd,
+};
+
+/* CSV format */
+static const CopyFromRoutine CopyFromRoutineCSV = {
+	.CopyFromInFunc = CopyFromTextLikeInFunc,
+	.CopyFromStart = CopyFromTextLikeStart,
+	.CopyFromOneRow = CopyFromCSVOneRow,
+	.CopyFromEnd = CopyFromTextLikeEnd,
+};
+
+/* BINARY format */
+static const CopyFromRoutine CopyFromRoutineBinary = {
+	.CopyFromInFunc = CopyFromBinaryInFunc,
+	.CopyFromStart = CopyFromBinaryStart,
+	.CopyFromOneRow = CopyFromBinaryOneRow,
+	.CopyFromEnd = CopyFromBinaryEnd,
+};
+
+/* Return COPY FROM routines for the given option */
+static const CopyFromRoutine *
+CopyFromGetRoutine(CopyFormatOptions opts)
 {
-	Oid			func_oid;
+	if (opts.csv_mode)
+		return &CopyFromRoutineCSV;
+	else if (opts.binary)
+		return &CopyFromRoutineBinary;
 
-	getTypeInputInfo(atttypid, &func_oid, typioparam);
-	fmgr_info(func_oid, finfo);
+	/* default is text */
+	return &CopyFromRoutineText;
 }
 
-/*
- * CopyFromTextLikeStart
- *
- * Start of COPY FROM for text/CSV format.
- */
+/* Implementation of the start callback for text and CSV formats */
 static void
 CopyFromTextLikeStart(CopyFromState cstate, TupleDesc tupDesc)
 {
@@ -162,24 +196,37 @@ CopyFromTextLikeStart(CopyFromState cstate, TupleDesc tupDesc)
 }
 
 /*
- * CopyFromTextLikeEnd
- *
- * End of COPY FROM for text/CSV format.
+ * Implementation of the infunc callback for text and CSV formats. Assign
+ * the input function data to the given *finfo.
  */
 static void
+CopyFromTextLikeInFunc(CopyFromState cstate, Oid atttypid, FmgrInfo *finfo,
+					   Oid *typioparam)
+{
+	Oid			func_oid;
+
+	getTypeInputInfo(atttypid, &func_oid, typioparam);
+	fmgr_info(func_oid, finfo);
+}
+
+/* Implementation of the end callback for text and CSV formats */
+static void
 CopyFromTextLikeEnd(CopyFromState cstate)
 {
 	/* nothing to do */
 }
 
-/*
- * CopyFromRoutine implementation for "binary".
- */
+/* Implementation of the start callback for binary format */
+static void
+CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc)
+{
+	/* Read and verify binary header */
+	ReceiveCopyBinaryHeader(cstate);
+}
 
 /*
- * CopyFromBinaryInFunc
- *
- * Assign input function data for a relation's attribute in binary format.
+ * Implementation of the infunc callback for binary format. Assign
+ * the binary input function to the given *finfo.
  */
 static void
 CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid,
@@ -191,72 +238,13 @@ CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid,
 	fmgr_info(func_oid, finfo);
 }
 
-/*
- * CopyFromBinaryStart
- *
- * Start of COPY FROM for binary format.
- */
-static void
-CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc)
-{
-	/* Read and verify binary header */
-	ReceiveCopyBinaryHeader(cstate);
-}
-
-/*
- * CopyFromBinaryEnd
- *
- * End of COPY FROM for binary format.
- */
+/* Implementation of the end callback for binary format */
 static void
 CopyFromBinaryEnd(CopyFromState cstate)
 {
 	/* nothing to do */
 }
 
-/*
- * Routines assigned to each format.
-+
- * CSV and text share the same implementation, at the exception of the
- * per-row callback.
- */
-static const CopyFromRoutine CopyFromRoutineText = {
-	.CopyFromInFunc = CopyFromTextLikeInFunc,
-	.CopyFromStart = CopyFromTextLikeStart,
-	.CopyFromOneRow = CopyFromTextOneRow,
-	.CopyFromEnd = CopyFromTextLikeEnd,
-};
-
-static const CopyFromRoutine CopyFromRoutineCSV = {
-	.CopyFromInFunc = CopyFromTextLikeInFunc,
-	.CopyFromStart = CopyFromTextLikeStart,
-	.CopyFromOneRow = CopyFromCSVOneRow,
-	.CopyFromEnd = CopyFromTextLikeEnd,
-};
-
-static const CopyFromRoutine CopyFromRoutineBinary = {
-	.CopyFromInFunc = CopyFromBinaryInFunc,
-	.CopyFromStart = CopyFromBinaryStart,
-	.CopyFromOneRow = CopyFromBinaryOneRow,
-	.CopyFromEnd = CopyFromBinaryEnd,
-};
-
-/*
- * Define the COPY FROM routines to use for a format.
- */
-static const CopyFromRoutine *
-CopyFromGetRoutine(CopyFormatOptions opts)
-{
-	if (opts.csv_mode)
-		return &CopyFromRoutineCSV;
-	else if (opts.binary)
-		return &CopyFromRoutineBinary;
-
-	/* default is text */
-	return &CopyFromRoutineText;
-}
-
-
 /*
  * error context callback for COPY FROM
  *
@@ -1578,7 +1566,7 @@ BeginCopyFrom(ParseState *pstate,
 	/* Extract options from the statement node tree */
 	ProcessCopyOptions(pstate, &cstate->opts, true /* is_from */ , options);
 
-	/* Set format routine */
+	/* Set the format routine */
 	cstate->routine = CopyFromGetRoutine(cstate->opts);
 
 	/* Process the target relation */
@@ -1918,6 +1906,7 @@ BeginCopyFrom(ParseState *pstate,
 void
 EndCopyFrom(CopyFromState cstate)
 {
+	/* Invoke the end callback */
 	cstate->routine->CopyFromEnd(cstate);
 
 	/* No COPY FROM related resources except memory. */
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 0447c4df7e..5416583e94 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -141,12 +141,14 @@ static const char BinarySignature[11] = "PGCOPY\n\377\r\n\0";
 
 /* non-export function prototypes */
 static bool CopyReadLine(CopyFromState cstate, bool is_csv);
-static pg_attribute_always_inline bool CopyReadLineText(CopyFromState cstate, bool is_csv);
+static bool CopyReadLineText(CopyFromState cstate, bool is_csv);
 static int	CopyReadAttributesText(CopyFromState cstate);
 static int	CopyReadAttributesCSV(CopyFromState cstate);
 static Datum CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo,
 									 Oid typioparam, int32 typmod,
 									 bool *isnull);
+static bool CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext,
+								   Datum *values, bool *nulls, bool is_csv);
 
 
 /* Low-level communications functions */
@@ -740,6 +742,8 @@ CopyReadBinaryData(CopyFromState cstate, char *dest, int nbytes)
  * in the relation.
  *
  * NOTE: force_not_null option are not applied to the returned fields.
+ *
+ * We use pg_attribute_always_inline to reduce function call overheads.
  */
 static pg_attribute_always_inline bool
 NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields, bool is_csv)
@@ -839,20 +843,30 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields, bool i
 	return true;
 }
 
+/* Implementation of the per-row callback for text format */
+bool
+CopyFromTextOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values,
+				   bool *nulls)
+{
+	return CopyFromTextLikeOneRow(cstate, econtext, values, nulls, false);
+}
+
+/* Implementation of the per-row callback for CSV format */
+bool
+CopyFromCSVOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values,
+				  bool *nulls)
+{
+	return CopyFromTextLikeOneRow(cstate, econtext, values, nulls, true);
+}
+
 /*
- * CopyFromTextLikeOneRow
- *
- * Copy one row to a set of `values` and `nulls` for the text and CSV
- * formats.
- *
  * Workhorse for CopyFromTextOneRow() and CopyFromCSVOneRow().
+ *
+ * We use pg_attribute_always_inline to reduce function call overheads.
  */
 static pg_attribute_always_inline bool
-CopyFromTextLikeOneRow(CopyFromState cstate,
-					   ExprContext *econtext,
-					   Datum *values,
-					   bool *nulls,
-					   bool is_csv)
+CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext,
+					   Datum *values, bool *nulls, bool is_csv)
 {
 	TupleDesc	tupDesc;
 	AttrNumber	attr_count;
@@ -1001,43 +1015,10 @@ CopyFromTextLikeOneRow(CopyFromState cstate,
 	return true;
 }
 
-
-/*
- * CopyFromTextOneRow
- *
- * Per-row callback for COPY FROM with text format.
- */
-bool
-CopyFromTextOneRow(CopyFromState cstate,
-				   ExprContext *econtext,
-				   Datum *values,
-				   bool *nulls)
-{
-	return CopyFromTextLikeOneRow(cstate, econtext, values, nulls, false);
-}
-
-/*
- * CopyFromCSVOneRow
- *
- * Per-row callback for COPY FROM with CSV format.
- */
-bool
-CopyFromCSVOneRow(CopyFromState cstate,
-				  ExprContext *econtext,
-				  Datum *values,
-				  bool *nulls)
-{
-	return CopyFromTextLikeOneRow(cstate, econtext, values, nulls, true);
-}
-
-/*
- * CopyFromBinaryOneRow
- *
- * Copy one row to a set of `values` and `nulls` for the binary format.
- */
+/* Implementation of the per-row callback for binary format */
 bool
-CopyFromBinaryOneRow(CopyFromState cstate, ExprContext *econtext,
-					 Datum *values, bool *nulls)
+CopyFromBinaryOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values,
+					 bool *nulls)
 {
 	TupleDesc	tupDesc;
 	AttrNumber	attr_count;
@@ -1130,6 +1111,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
 	MemSet(nulls, true, num_phys_attrs * sizeof(bool));
 	MemSet(cstate->defaults, false, num_phys_attrs * sizeof(bool));
 
+	/* Get one row from source */
 	if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls))
 		return false;
 
@@ -1237,7 +1219,7 @@ CopyReadLine(CopyFromState cstate, bool is_csv)
 /*
  * CopyReadLineText - inner loop of CopyReadLine for text mode
  */
-static pg_attribute_always_inline bool
+static bool
 CopyReadLineText(CopyFromState cstate, bool is_csv)
 {
 	char	   *copy_input_buf;
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
index 224fda172e..ff269def9d 100644
--- a/src/include/commands/copyapi.h
+++ b/src/include/commands/copyapi.h
@@ -63,38 +63,42 @@ typedef struct CopyToRoutine
 typedef struct CopyFromRoutine
 {
 	/*
-	 * Called when COPY FROM is started to set up the input functions
-	 * associated with 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.
+	 * Set input function information. This callback is called once at the
+	 * beginning of COPY FROM.
+	 *
+	 * '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.
+	 * Start a COPY FROM. This callback is called once at the beginning of
+	 * COPY FROM.
 	 *
-	 * `tupDesc` is the tuple descriptor of the relation where the data needs
+	 * '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.
+	 * Read one row from the source and fill *values and *nulls.
 	 *
 	 * '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.
+	 * Returns false if there are no more tuples to read.
 	 */
 	bool		(*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
 								   Datum *values, bool *nulls);
 
-	/* Called when COPY FROM has ended. */
+	/* End a COPY FROM. This callback is called once at the end of COPY FROM */
 	void		(*CopyFromEnd) (CopyFromState cstate);
 } CopyFromRoutine;
 
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index c11b5ff3cc..55fe24d728 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -187,7 +187,7 @@ typedef struct CopyFromStateData
 extern void ReceiveCopyBegin(CopyFromState cstate);
 extern void ReceiveCopyBinaryHeader(CopyFromState cstate);
 
-/* Callbacks for CopyFromRoutine->CopyFromOneRow */
+/* One-row callbacks for built-in formats defined in copyfromparse.c */
 extern bool CopyFromTextOneRow(CopyFromState cstate, ExprContext *econtext,
 							   Datum *values, bool *nulls);
 extern bool CopyFromCSVOneRow(CopyFromState cstate, ExprContext *econtext,
-- 
2.43.5