Re: Make COPY format extendable: Extract COPY TO format implementations

Hi,

In <20240209192705.5qdilvviq3py2voq@awork3.anarazel.de>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 9 Feb 2024 11:27:05 -0800,
  Andres Freund <andres@anarazel.de> wrote:

>> +static void
>> +CopyFromTextInFunc(CopyFromState cstate, Oid atttypid,
>> +                   FmgrInfo *finfo, Oid *typioparam)
>> +{
>> +    Oid            func_oid;
>> +
>> +    getTypeInputInfo(atttypid, &func_oid, typioparam);
>> +    fmgr_info(func_oid, finfo);
>> +}
> 
> FWIW, we should really change the copy code to initialize FunctionCallInfoData
> instead of re-initializing that on every call, realy makes a difference
> performance wise.

How about the attached patch approach? If it's a desired
approach, I can also write a separated patch for COPY TO.

>> +    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;
>> +}
> 
> Isn't this precisely repeating the mistake of 2889fd23be56?

What do you think about the approach in my previous mail's
attachments?

https://www.postgresql.org/message-id/flat/20240209.163205.704848659612151781.kou%40clear-code.com#dbb1f8d7f2f0e8fe3c7e37a757fcfc54

If it's a desired approach, I can prepare a v15 patch set
based on the v14 patch set and the approach.


I'll reply other comments later...


Thanks,
-- 
kou
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 41f6bc43e4..a43c853e99 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1691,6 +1691,10 @@ BeginCopyFrom(ParseState *pstate,
     /* We keep those variables in cstate. */
     cstate->in_functions = in_functions;
     cstate->typioparams = typioparams;
+    if (cstate->opts.binary)
+        cstate->fcinfo = PrepareInputFunctionCallInfo();
+    else
+        cstate->fcinfo = PrepareReceiveFunctionCallInfo();
     cstate->defmap = defmap;
     cstate->defexprs = defexprs;
     cstate->volatile_defexprs = volatile_defexprs;
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 906756362e..e372e5efb8 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -853,6 +853,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
                 num_defaults = cstate->num_defaults;
     FmgrInfo   *in_functions = cstate->in_functions;
     Oid           *typioparams = cstate->typioparams;
+    FunctionCallInfoBaseData *fcinfo = cstate->fcinfo;
     int            i;
     int           *defmap = cstate->defmap;
     ExprState **defexprs = cstate->defexprs;
@@ -953,12 +954,13 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
              * If ON_ERROR is specified with IGNORE, skip rows with soft
              * errors
              */
-            else if (!InputFunctionCallSafe(&in_functions[m],
-                                            string,
-                                            typioparams[m],
-                                            att->atttypmod,
-                                            (Node *) cstate->escontext,
-                                            &values[m]))
+            else if (!PreparedInputFunctionCallSafe(fcinfo,
+                                                    &in_functions[m],
+                                                    string,
+                                                    typioparams[m],
+                                                    att->atttypmod,
+                                                    (Node *) cstate->escontext,
+                                                    &values[m]))
             {
                 cstate->num_errors++;
                 return true;
@@ -1958,7 +1960,7 @@ CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo,
     if (fld_size == -1)
     {
         *isnull = true;
-        return ReceiveFunctionCall(flinfo, NULL, typioparam, typmod);
+        return PreparedReceiveFunctionCall(cstate->fcinfo, flinfo, NULL, typioparam, typmod);
     }
     if (fld_size < 0)
         ereport(ERROR,
@@ -1979,8 +1981,8 @@ CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo,
     cstate->attribute_buf.data[fld_size] = '\0';
 
     /* Call the column type's binary input converter */
-    result = ReceiveFunctionCall(flinfo, &cstate->attribute_buf,
-                                 typioparam, typmod);
+    result = PreparedReceiveFunctionCall(cstate->fcinfo, flinfo, &cstate->attribute_buf,
+                                         typioparam, typmod);
 
     /* Trouble if it didn't eat the whole buffer */
     if (cstate->attribute_buf.cursor != cstate->attribute_buf.len)
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index e48a86be54..b0b5310219 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -1672,6 +1672,73 @@ DirectInputFunctionCallSafe(PGFunction func, char *str,
     return true;
 }
 
+/*
+ * Prepare callinfo for PreparedInputFunctionCallSafe to reuse one callinfo
+ * instead of initializing it for each call. This is for performance.
+ */
+FunctionCallInfoBaseData *
+PrepareInputFunctionCallInfo(void)
+{
+    FunctionCallInfoBaseData *fcinfo;
+
+    fcinfo = (FunctionCallInfoBaseData *) palloc(SizeForFunctionCallInfo(3));
+    InitFunctionCallInfoData(*fcinfo, NULL, 3, InvalidOid, NULL, NULL);
+    fcinfo->args[0].isnull = false;
+    fcinfo->args[1].isnull = false;
+    fcinfo->args[2].isnull = false;
+    return fcinfo;
+}
+
+/*
+ * Call a previously-looked-up datatype input function, with prepared callinfo
+ * and non-exception handling of "soft" errors.
+ *
+ * This is basically like InputFunctionCallSafe, but it reuses prepared
+ * callinfo.
+ */
+bool
+PreparedInputFunctionCallSafe(FunctionCallInfoBaseData *fcinfo,
+                              FmgrInfo *flinfo, char *str,
+                              Oid typioparam, int32 typmod,
+                              fmNodePtr escontext,
+                              Datum *result)
+{
+    if (str == NULL && flinfo->fn_strict)
+    {
+        *result = (Datum) 0;    /* just return null result */
+        return true;
+    }
+
+    fcinfo->flinfo = flinfo;
+    fcinfo->context = escontext;
+    fcinfo->isnull = false;
+    fcinfo->args[0].value = CStringGetDatum(str);
+    fcinfo->args[1].value = ObjectIdGetDatum(typioparam);
+    fcinfo->args[2].value = Int32GetDatum(typmod);
+
+    *result = FunctionCallInvoke(fcinfo);
+
+    /* Result value is garbage, and could be null, if an error was reported */
+    if (SOFT_ERROR_OCCURRED(escontext))
+        return false;
+
+    /* Otherwise, should get null result if and only if str is NULL */
+    if (str == NULL)
+    {
+        if (!fcinfo->isnull)
+            elog(ERROR, "input function %u returned non-NULL",
+                 flinfo->fn_oid);
+    }
+    else
+    {
+        if (fcinfo->isnull)
+            elog(ERROR, "input function %u returned NULL",
+                 flinfo->fn_oid);
+    }
+
+    return true;
+}
+
 /*
  * Call a previously-looked-up datatype output function.
  *
@@ -1731,6 +1798,65 @@ ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf,
     return result;
 }
 
+/*
+ * Prepare callinfo for PreparedReceiveFunctionCall to reuse one callinfo
+ * instead of initializing it for each call. This is for performance.
+ */
+FunctionCallInfoBaseData *
+PrepareReceiveFunctionCallInfo(void)
+{
+    FunctionCallInfoBaseData *fcinfo;
+
+    fcinfo = (FunctionCallInfoBaseData *) palloc(SizeForFunctionCallInfo(3));
+    InitFunctionCallInfoData(*fcinfo, NULL, 3, InvalidOid, NULL, NULL);
+    fcinfo->args[0].isnull = false;
+    fcinfo->args[1].isnull = false;
+    fcinfo->args[2].isnull = false;
+    return fcinfo;
+}
+
+/*
+ * Call a previously-looked-up datatype binary-input function, with prepared
+ * callinfo.
+ *
+ * This is basically like ReceiveFunctionCall, but it reuses prepared
+ * callinfo.
+ */
+Datum
+PreparedReceiveFunctionCall(FunctionCallInfoBaseData *fcinfo,
+                            FmgrInfo *flinfo, StringInfo buf,
+                            Oid typioparam, int32 typmod)
+{
+    Datum        result;
+
+    if (buf == NULL && flinfo->fn_strict)
+        return (Datum) 0;        /* just return null result */
+
+    fcinfo->flinfo = flinfo;
+    fcinfo->isnull = false;
+    fcinfo->args[0].value = PointerGetDatum(buf);
+    fcinfo->args[1].value = ObjectIdGetDatum(typioparam);
+    fcinfo->args[2].value = Int32GetDatum(typmod);
+
+    result = FunctionCallInvoke(fcinfo);
+
+    /* Should get null result if and only if buf is NULL */
+    if (buf == NULL)
+    {
+        if (!fcinfo->isnull)
+            elog(ERROR, "receive function %u returned non-NULL",
+                 flinfo->fn_oid);
+    }
+    else
+    {
+        if (fcinfo->isnull)
+            elog(ERROR, "receive function %u returned NULL",
+                 flinfo->fn_oid);
+    }
+
+    return result;
+}
+
 /*
  * Call a previously-looked-up datatype binary-output function.
  *
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index 759f8e3d09..4d7928b3ac 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -104,6 +104,7 @@ typedef struct CopyFromStateData
     Oid           *typioparams;    /* array of element types for in_functions */
     ErrorSaveContext *escontext;    /* soft error trapper during in_functions
                                      * execution */
+    FunctionCallInfoBaseData *fcinfo;    /* reusable callinfo for in_functions */
     uint64        num_errors;        /* total number of rows which contained soft
                                  * errors */
     int           *defmap;            /* array of default att numbers related to
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index ccb4070a25..994d8ce487 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -708,12 +708,24 @@ extern bool DirectInputFunctionCallSafe(PGFunction func, char *str,
                                         Oid typioparam, int32 typmod,
                                         fmNodePtr escontext,
                                         Datum *result);
+extern FunctionCallInfoBaseData *PrepareInputFunctionCallInfo(void);
+extern bool
+            PreparedInputFunctionCallSafe(FunctionCallInfoBaseData *fcinfo,
+                                          FmgrInfo *flinfo, char *str,
+                                          Oid typioparam, int32 typmod,
+                                          fmNodePtr escontext,
+                                          Datum *result);
 extern Datum OidInputFunctionCall(Oid functionId, char *str,
                                   Oid typioparam, int32 typmod);
 extern char *OutputFunctionCall(FmgrInfo *flinfo, Datum val);
 extern char *OidOutputFunctionCall(Oid functionId, Datum val);
 extern Datum ReceiveFunctionCall(FmgrInfo *flinfo, fmStringInfo buf,
                                  Oid typioparam, int32 typmod);
+extern FunctionCallInfoBaseData *PrepareReceiveFunctionCallInfo(void);
+extern Datum
+            PreparedReceiveFunctionCall(FunctionCallInfoBaseData *fcinfo,
+                                        FmgrInfo *flinfo, fmStringInfo buf,
+                                        Oid typioparam, int32 typmod);
 extern Datum OidReceiveFunctionCall(Oid functionId, fmStringInfo buf,
                                     Oid typioparam, int32 typmod);
 extern bytea *SendFunctionCall(FmgrInfo *flinfo, Datum val);