Re: Make COPY format extendable: Extract COPY TO format implementations - Mailing list pgsql-hackers

Hi,

In <ZcWoTr1N0GELFA9E@paquier.xyz>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 9 Feb 2024 13:21:34 +0900,
  Michael Paquier <michael@paquier.xyz> wrote:

>> - Revisit what we have here, looking at more profiles to see how HEAD
>> an v13 compare.  It looks like we are on a good path, but let's tackle
>> things one step at a time.
> 
> And attached is a v14 that's rebased on HEAD.

Thanks!

> A next step I think we could take is to split the binary-only and the
> text/csv-only data in each cstate into their own structure to make the
> structure, with an opaque pointer that custom formats could use, but a
> lot of fields are shared as well.

It'll make COPY code base cleaner but it may decrease
performance. How about just adding an opaque pointer to each
cstate as the next step and then try the split?

My suggestion:
1. Introduce Copy{To,From}Routine
   (We can do it based on the v14 patch.)
2. Add an opaque pointer to Copy{To,From}Routine
   (This must not have performance impact.)
3.a. Split format specific data to the opaque space
3.b. Add support for registering custom format handler by
     creating a function
4. ...

>                                    This patch is already complicated
> enough IMO, so I'm OK to leave it out for the moment, and focus on
> making this infra pluggable as a next step.

I agree with you.

> Are there any comments about this v14?  Sutou-san?

Here are my comments:


+    /* Set read attribute callback */
+    if (cstate->opts.csv_mode)
+        cstate->copy_read_attributes = CopyReadAttributesCSV;
+    else
+        cstate->copy_read_attributes = CopyReadAttributesText;

I think that we should not use this approach for
performance. We need to use "static inline" and constant
argument instead something like the attached
remove-copy-read-attributes.diff.

We have similar codes for
CopyReadLine()/CopyReadLineText(). The attached
remove-copy-read-attributes-and-optimize-copy-read-line.diff
also applies the same optimization to
CopyReadLine()/CopyReadLineText().

I hope that this improved performance of COPY FROM.

+/*
+ * Routines assigned to each format.
++

Garbage "+"

+ * CSV and text share the same implementation, at the exception of the
+ * copy_read_attributes callback.
+ */


+/*
+ * CopyToTextOneRow
+ *
+ * Process one row for text/CSV format.
+ */
+static void
+CopyToTextOneRow(CopyToState cstate,
+                 TupleTableSlot *slot)
+{
...
+            if (cstate->opts.csv_mode)
+                CopyAttributeOutCSV(cstate, string,
+                                    cstate->opts.force_quote_flags[attnum - 1]);
+            else
+                CopyAttributeOutText(cstate, string);
...

How about use "static inline" and constant argument approach
here too?

static inline void
CopyToTextBasedOneRow(CopyToState cstate,
                      TupleTableSlot *slot,
                      bool csv_mode)
{
...
            if (cstate->opts.csv_mode)
                CopyAttributeOutCSV(cstate, string,
                                    cstate->opts.force_quote_flags[attnum - 1]);
            else
                CopyAttributeOutText(cstate, string);
...
}

static void
CopyToTextOneRow(CopyToState cstate,
                 TupleTableSlot *slot,
                 bool csv_mode)
{
    CopyToTextBasedOneRow(cstate, slot, false);
}

static void
CopyToCSVOneRow(CopyToState cstate,
                TupleTableSlot *slot,
                bool csv_mode)
{
    CopyToTextBasedOneRow(cstate, slot, true);
}

static const CopyToRoutine CopyCSVRoutineText = {
    ...
    .CopyToOneRow = CopyToCSVOneRow,
    ...
};


Thanks,
-- 
kou
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..1f8b2ddc6e 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
@@ -152,6 +152,8 @@ 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 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;
@@ -773,7 +780,10 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
         {
             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,
@@ -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 */
@@ -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
  *
@@ -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);

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);