remove-copy-read-attributes-and-optimize-copy-read-line.diff

text/x-patch

Filename: remove-copy-read-attributes-and-optimize-copy-read-line.diff
Type: text/x-patch
Part: 1
Message: Re: Make COPY format extendable: Extract COPY TO format implementations

Patch

Format: unified
File+
src/backend/commands/copyfrom.c 2 9
src/backend/commands/copyfromparse.c 93 42
src/include/commands/copyfrom_internal.h 2 10
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index a90b7189b5..6e244fb443 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -158,12 +158,6 @@ CopyFromTextStart(CopyFromState cstate, TupleDesc tupDesc)
 	attr_count = list_length(cstate->attnumlist);
 	cstate->max_fields = attr_count;
 	cstate->raw_fields = (char **) palloc(attr_count * sizeof(char *));
-
-	/* Set read attribute callback */
-	if (cstate->opts.csv_mode)
-		cstate->copy_read_attributes = CopyReadAttributesCSV;
-	else
-		cstate->copy_read_attributes = CopyReadAttributesText;
 }
 
 /*
@@ -221,9 +215,8 @@ CopyFromBinaryEnd(CopyFromState cstate)
 
 /*
  * Routines assigned to each format.
-+
  * CSV and text share the same implementation, at the exception of the
- * copy_read_attributes callback.
+ * CopyFromOneRow callback.
  */
 static const CopyFromRoutine CopyFromRoutineText = {
 	.CopyFromInFunc = CopyFromTextInFunc,
@@ -235,7 +228,7 @@ static const CopyFromRoutine CopyFromRoutineText = {
 static const CopyFromRoutine CopyFromRoutineCSV = {
 	.CopyFromInFunc = CopyFromTextInFunc,
 	.CopyFromStart = CopyFromTextStart,
-	.CopyFromOneRow = CopyFromTextOneRow,
+	.CopyFromOneRow = CopyFromCSVOneRow,
 	.CopyFromEnd = CopyFromTextEnd,
 };
 
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index c45f9ae134..ea2eb45491 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -25,10 +25,10 @@
  *    is copied into 'line_buf', with quotes and escape characters still
  *    intact.
  *
- * 4. CopyReadAttributesText/CSV() function (via copy_read_attribute) takes
- *    the input line from 'line_buf', and splits it into fields, unescaping
- *    the data as required.  The fields are stored in 'attribute_buf', and
- *    'raw_fields' array holds pointers to each field.
+ * 4. CopyReadAttributesText/CSV() function takes the input line from
+ *    'line_buf', and splits it into fields, unescaping the data as required.
+ *    The fields are stored in 'attribute_buf', and 'raw_fields' array holds
+ *    pointers to each field.
  *
  * If encoding conversion is not required, a shortcut is taken in step 2 to
  * avoid copying the data unnecessarily.  The 'input_buf' pointer is set to
@@ -150,8 +150,10 @@ static const char BinarySignature[11] = "PGCOPY\n\377\r\n\0";
 
 
 /* non-export function prototypes */
-static bool CopyReadLine(CopyFromState cstate);
-static bool CopyReadLineText(CopyFromState cstate);
+static inline bool CopyReadLine(CopyFromState cstate, bool csv_mode);
+static inline bool CopyReadLineText(CopyFromState cstate, bool csv_mode);
+static int	CopyReadAttributesText(CopyFromState cstate);
+static int	CopyReadAttributesCSV(CopyFromState cstate);
 static Datum CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo,
 									 Oid typioparam, int32 typmod,
 									 bool *isnull);
@@ -748,9 +750,14 @@ CopyReadBinaryData(CopyFromState cstate, char *dest, int nbytes)
  * in the relation.
  *
  * NOTE: force_not_null option are not applied to the returned fields.
+ *
+ * Creating static inline NextCopyFromRawFieldsInternal() and call this with
+ * constant 'csv_mode' value from CopyFromTextOneRow()/CopyFromCSVOneRow()
+ * (via CopyFromTextBasedOneRow()) is for optimization. We can avoid indirect
+ * function call by this.
  */
-bool
-NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
+static inline bool
+NextCopyFromRawFieldsInternal(CopyFromState cstate, char ***fields, int *nfields, bool csv_mode)
 {
 	int			fldct;
 	bool		done;
@@ -767,13 +774,16 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
 		tupDesc = RelationGetDescr(cstate->rel);
 
 		cstate->cur_lineno++;
-		done = CopyReadLine(cstate);
+		done = CopyReadLine(cstate, csv_mode);
 
 		if (cstate->opts.header_line == COPY_HEADER_MATCH)
 		{
 			int			fldnum;
 
-			fldct = cstate->copy_read_attributes(cstate);
+			if (csv_mode)
+				fldct = CopyReadAttributesCSV(cstate);
+			else
+				fldct = CopyReadAttributesText(cstate);
 
 			if (fldct != list_length(cstate->attnumlist))
 				ereport(ERROR,
@@ -814,7 +824,7 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
 	cstate->cur_lineno++;
 
 	/* Actually read the line into memory here */
-	done = CopyReadLine(cstate);
+	done = CopyReadLine(cstate, csv_mode);
 
 	/*
 	 * EOF at start of line means we're done.  If we see EOF after some
@@ -825,7 +835,10 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
 		return false;
 
 	/* Parse the line into de-escaped field values */
-	fldct = cstate->copy_read_attributes(cstate);
+	if (csv_mode)
+		fldct = CopyReadAttributesCSV(cstate);
+	else
+		fldct = CopyReadAttributesText(cstate);
 
 	*fields = cstate->raw_fields;
 	*nfields = fldct;
@@ -833,16 +846,26 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
 }
 
 /*
- * CopyFromTextOneRow
+ * See NextCopyFromRawFieldsInternal() for details.
+ */
+bool
+NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
+{
+	return NextCopyFromRawFieldsInternal(cstate, fields, nfields, cstate->opts.csv_mode);
+}
+
+/*
+ * CopyFromTextBasedOneRow
  *
  * Copy one row to a set of `values` and `nulls` for the text and CSV
  * formats.
  */
-bool
-CopyFromTextOneRow(CopyFromState cstate,
-				   ExprContext *econtext,
-				   Datum *values,
-				   bool *nulls)
+static inline bool
+CopyFromTextBasedOneRow(CopyFromState cstate,
+						ExprContext *econtext,
+						Datum *values,
+						bool *nulls,
+						bool csv_mode)
 {
 	TupleDesc	tupDesc;
 	AttrNumber	attr_count;
@@ -859,7 +882,7 @@ CopyFromTextOneRow(CopyFromState cstate,
 	attr_count = list_length(cstate->attnumlist);
 
 	/* read raw fields in the next line */
-	if (!NextCopyFromRawFields(cstate, &field_strings, &fldct))
+	if (!NextCopyFromRawFieldsInternal(cstate, &field_strings, &fldct, csv_mode))
 		return false;
 
 	/* check for overflowing fields */
@@ -894,7 +917,7 @@ CopyFromTextOneRow(CopyFromState cstate,
 		cstate->cur_attname = NameStr(att->attname);
 		cstate->cur_attval = string;
 
-		if (cstate->opts.csv_mode)
+		if (csv_mode)
 		{
 			if (string == NULL &&
 				cstate->opts.force_notnull_flags[m])
@@ -956,6 +979,34 @@ CopyFromTextOneRow(CopyFromState cstate,
 	return true;
 }
 
+/*
+ * CopyFromTextOneRow
+ *
+ * Copy one row to a set of `values` and `nulls` for the text format.
+ */
+bool
+CopyFromTextOneRow(CopyFromState cstate,
+				   ExprContext *econtext,
+				   Datum *values,
+				   bool *nulls)
+{
+	return CopyFromTextBasedOneRow(cstate, econtext, values, nulls, false);
+}
+
+/*
+ * CopyFromCSVOneRow
+ *
+ * Copy one row to a set of `values` and `nulls` for the CSV format.
+ */
+bool
+CopyFromCSVOneRow(CopyFromState cstate,
+				  ExprContext *econtext,
+				  Datum *values,
+				  bool *nulls)
+{
+	return CopyFromTextBasedOneRow(cstate, econtext, values, nulls, true);
+}
+
 /*
  * CopyFromBinaryOneRow
  *
@@ -1089,8 +1140,8 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
  * by newline.  The terminating newline or EOF marker is not included
  * in the final value of line_buf.
  */
-static bool
-CopyReadLine(CopyFromState cstate)
+static inline bool
+CopyReadLine(CopyFromState cstate, bool csv_mode)
 {
 	bool		result;
 
@@ -1098,7 +1149,7 @@ CopyReadLine(CopyFromState cstate)
 	cstate->line_buf_valid = false;
 
 	/* Parse data and transfer into line_buf */
-	result = CopyReadLineText(cstate);
+	result = CopyReadLineText(cstate, csv_mode);
 
 	if (result)
 	{
@@ -1165,8 +1216,8 @@ CopyReadLine(CopyFromState cstate)
 /*
  * CopyReadLineText - inner loop of CopyReadLine for text mode
  */
-static bool
-CopyReadLineText(CopyFromState cstate)
+static inline bool
+CopyReadLineText(CopyFromState cstate, bool csv_mode)
 {
 	char	   *copy_input_buf;
 	int			input_buf_ptr;
@@ -1182,7 +1233,7 @@ CopyReadLineText(CopyFromState cstate)
 	char		quotec = '\0';
 	char		escapec = '\0';
 
-	if (cstate->opts.csv_mode)
+	if (csv_mode)
 	{
 		quotec = cstate->opts.quote[0];
 		escapec = cstate->opts.escape[0];
@@ -1262,7 +1313,7 @@ CopyReadLineText(CopyFromState cstate)
 		prev_raw_ptr = input_buf_ptr;
 		c = copy_input_buf[input_buf_ptr++];
 
-		if (cstate->opts.csv_mode)
+		if (csv_mode)
 		{
 			/*
 			 * If character is '\\' or '\r', we may need to look ahead below.
@@ -1301,7 +1352,7 @@ CopyReadLineText(CopyFromState cstate)
 		}
 
 		/* Process \r */
-		if (c == '\r' && (!cstate->opts.csv_mode || !in_quote))
+		if (c == '\r' && (!csv_mode || !in_quote))
 		{
 			/* Check for \r\n on first line, _and_ handle \r\n. */
 			if (cstate->eol_type == EOL_UNKNOWN ||
@@ -1329,10 +1380,10 @@ CopyReadLineText(CopyFromState cstate)
 					if (cstate->eol_type == EOL_CRNL)
 						ereport(ERROR,
 								(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
-								 !cstate->opts.csv_mode ?
+								 !csv_mode ?
 								 errmsg("literal carriage return found in data") :
 								 errmsg("unquoted carriage return found in data"),
-								 !cstate->opts.csv_mode ?
+								 !csv_mode ?
 								 errhint("Use \"\\r\" to represent carriage return.") :
 								 errhint("Use quoted CSV field to represent carriage return.")));
 
@@ -1346,10 +1397,10 @@ CopyReadLineText(CopyFromState cstate)
 			else if (cstate->eol_type == EOL_NL)
 				ereport(ERROR,
 						(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
-						 !cstate->opts.csv_mode ?
+						 !csv_mode ?
 						 errmsg("literal carriage return found in data") :
 						 errmsg("unquoted carriage return found in data"),
-						 !cstate->opts.csv_mode ?
+						 !csv_mode ?
 						 errhint("Use \"\\r\" to represent carriage return.") :
 						 errhint("Use quoted CSV field to represent carriage return.")));
 			/* If reach here, we have found the line terminator */
@@ -1357,15 +1408,15 @@ CopyReadLineText(CopyFromState cstate)
 		}
 
 		/* Process \n */
-		if (c == '\n' && (!cstate->opts.csv_mode || !in_quote))
+		if (c == '\n' && (!csv_mode || !in_quote))
 		{
 			if (cstate->eol_type == EOL_CR || cstate->eol_type == EOL_CRNL)
 				ereport(ERROR,
 						(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
-						 !cstate->opts.csv_mode ?
+						 !csv_mode ?
 						 errmsg("literal newline found in data") :
 						 errmsg("unquoted newline found in data"),
-						 !cstate->opts.csv_mode ?
+						 !csv_mode ?
 						 errhint("Use \"\\n\" to represent newline.") :
 						 errhint("Use quoted CSV field to represent newline.")));
 			cstate->eol_type = EOL_NL;	/* in case not set yet */
@@ -1377,7 +1428,7 @@ CopyReadLineText(CopyFromState cstate)
 		 * In CSV mode, we only recognize \. alone on a line.  This is because
 		 * \. is a valid CSV data value.
 		 */
-		if (c == '\\' && (!cstate->opts.csv_mode || first_char_in_line))
+		if (c == '\\' && (!csv_mode || first_char_in_line))
 		{
 			char		c2;
 
@@ -1410,7 +1461,7 @@ CopyReadLineText(CopyFromState cstate)
 
 					if (c2 == '\n')
 					{
-						if (!cstate->opts.csv_mode)
+						if (!csv_mode)
 							ereport(ERROR,
 									(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
 									 errmsg("end-of-copy marker does not match previous newline style")));
@@ -1419,7 +1470,7 @@ CopyReadLineText(CopyFromState cstate)
 					}
 					else if (c2 != '\r')
 					{
-						if (!cstate->opts.csv_mode)
+						if (!csv_mode)
 							ereport(ERROR,
 									(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
 									 errmsg("end-of-copy marker corrupt")));
@@ -1435,7 +1486,7 @@ CopyReadLineText(CopyFromState cstate)
 
 				if (c2 != '\r' && c2 != '\n')
 				{
-					if (!cstate->opts.csv_mode)
+					if (!csv_mode)
 						ereport(ERROR,
 								(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
 								 errmsg("end-of-copy marker corrupt")));
@@ -1464,7 +1515,7 @@ CopyReadLineText(CopyFromState cstate)
 				result = true;	/* report EOF */
 				break;
 			}
-			else if (!cstate->opts.csv_mode)
+			else if (!csv_mode)
 			{
 				/*
 				 * If we are here, it means we found a backslash followed by
@@ -1530,7 +1581,7 @@ GetDecimalFromHex(char hex)
  *
  * The return value is the number of fields actually read.
  */
-int
+static int
 CopyReadAttributesText(CopyFromState cstate)
 {
 	char		delimc = cstate->opts.delim[0];
@@ -1784,7 +1835,7 @@ CopyReadAttributesText(CopyFromState cstate)
  * CopyReadAttributesText, except we parse the fields according to
  * "standard" (i.e. common) CSV usage.
  */
-int
+static int
 CopyReadAttributesCSV(CopyFromState cstate)
 {
 	char		delimc = cstate->opts.delim[0];
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index 5fb52dc629..5d597a3c8e 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -141,12 +141,6 @@ typedef struct CopyFromStateData
 	int			max_fields;
 	char	  **raw_fields;
 
-	/*
-	 * Per-format callback to parse lines, then fill raw_fields and
-	 * attribute_buf.
-	 */
-	CopyReadAttributes copy_read_attributes;
-
 	/*
 	 * Similarly, line_buf holds the whole input line being processed. The
 	 * input cycle is first to read the whole line into line_buf, and then
@@ -200,13 +194,11 @@ typedef struct CopyFromStateData
 extern void ReceiveCopyBegin(CopyFromState cstate);
 extern void ReceiveCopyBinaryHeader(CopyFromState cstate);
 
-/* Callbacks for copy_read_attributes */
-extern int	CopyReadAttributesCSV(CopyFromState cstate);
-extern int	CopyReadAttributesText(CopyFromState cstate);
-
 /* Callbacks for CopyFromRoutine->CopyFromOneRow */
 extern bool CopyFromTextOneRow(CopyFromState cstate, ExprContext *econtext,
 							   Datum *values, bool *nulls);
+extern bool CopyFromCSVOneRow(CopyFromState cstate, ExprContext *econtext,
+							  Datum *values, bool *nulls);
 extern bool CopyFromBinaryOneRow(CopyFromState cstate, ExprContext *econtext,
 								 Datum *values, bool *nulls);