Re: Why is pq_begintypsend so slow?

From: Sutou Kouhei <kou(at)clear-code(dot)com>
To: andres(at)anarazel(dot)de
Cc: robertmhaas(at)gmail(dot)com, laurenz(dot)albe(at)cybertec(dot)at, tgl(at)sss(dot)pgh(dot)pa(dot)us, jack(at)jncsoftware(dot)com, david(at)fetter(dot)org, jeff(dot)janes(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Why is pq_begintypsend so slow?
Date: 2024-02-18 08:38:09
Message-ID: 20240218.173809.707692226756628288.kou@clear-code.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

In <20240218015955(dot)rmw5mcmobt5hbene(at)awork3(dot)anarazel(dot)de>
"Re: Why is pq_begintypsend so slow?" on Sat, 17 Feb 2024 17:59:55 -0800,
Andres Freund <andres(at)anarazel(dot)de> wrote:

> v3-0008-wip-make-in-out-send-recv-calls-in-copy.c-cheaper.patch

It seems that this is an alternative approach of [1].

[1] https://www.postgresql.org/message-id/flat/20240215.153421.96888103784986803.kou%40clear-code.com#34df359e6d255795d16814ce138cc995

+typedef struct CopyInAttributeInfo
+{
+ AttrNumber num;
+ const char *name;
+
+ Oid typioparam;
+ int32 typmod;
+
+ FmgrInfo in_finfo;
+ union
+ {
+ FunctionCallInfoBaseData fcinfo;
+ char fcinfo_data[SizeForFunctionCallInfo(3)];
+ } in_fcinfo;

Do we need one FunctionCallInfoBaseData for each attribute?
How about sharing one FunctionCallInfoBaseData by all
attributes like [1]?

@@ -956,20 +956,47 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,

values[m] = ExecEvalExpr(defexprs[m], econtext, &nulls[m]);
}
-
- /*
- * 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]))

Inlining InputFuncallCallSafe() here to use pre-initialized
fcinfo will decrease maintainability. Because we abstract
InputFunctionCall family in fmgr.c. If we define a
InputFunctionCall variant here, we need to change both of
fmgr.c and here when InputFunctionCall family is changed.
How about defining details in fmgr.c and call it here
instead like [1]?

+ fcinfo->args[0].value = CStringGetDatum(string);
+ fcinfo->args[0].isnull = false;
+ fcinfo->args[1].value = ObjectIdGetDatum(attr->typioparam);
+ fcinfo->args[1].isnull = false;
+ fcinfo->args[2].value = Int32GetDatum(attr->typmod);
+ fcinfo->args[2].isnull = false;

I think that "fcinfo->isnull = false;" is also needed like
[1].

@@ -1966,7 +1992,7 @@ CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo,
if (fld_size == -1)
{
*isnull = true;
- return ReceiveFunctionCall(flinfo, NULL, typioparam, typmod);
+ return ReceiveFunctionCall(fcinfo->flinfo, NULL, attr->typioparam, attr->typmod);

Why pre-initialized fcinfo isn't used here?

Thanks,
--
kou

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andy Fan 2024-02-18 09:07:50 Re: Properly pathify the union planner
Previous Message Andy Fan 2024-02-18 08:19:04 Re: serial not accepted as datatype in ALTER TABLE ... ALTER COLUMN