Hi,
In <20240209192705.5qdilvviq3py2...@awork3.anarazel.de>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on
Fri, 9 Feb 2024 11:27:05 -0800,
Andres Freund <and...@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);
