From 1aa8324b81fa4979ffdc6ccf81d560eac9446948 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 7 Feb 2024 08:04:26 +0900 Subject: [PATCH] Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO" This reverts commit 2889fd23be56, following a discussion with Andres Freund as this callback, being called once per attribute when sending a relation's row, can involve a lot of indirect function calls (more attributes to deal with means more impact). The effects of a dispatch at this level would become more visible when improving the per-row code execution of COPY TO, impacting future potential performance improvements. Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de --- src/backend/commands/copyto.c | 47 +++++++++++------------------------ 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 52b42f8a52..d3dc3fc854 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -54,14 +54,6 @@ typedef enum CopyDest COPY_CALLBACK, /* to callback function */ } CopyDest; -/* - * Per-format callback to send output representation of one attribute for - * a `string`. `use_quote` tracks if quotes are required in the output - * representation. - */ -typedef void (*CopyAttributeOut) (CopyToState cstate, const char *string, - bool use_quote); - /* * This struct contains all the state variables used throughout a COPY TO * operation. @@ -105,7 +97,6 @@ typedef struct CopyToStateData MemoryContext copycontext; /* per-copy execution context */ FmgrInfo *out_functions; /* lookup info for output functions */ - CopyAttributeOut copy_attribute_out; /* output representation callback */ MemoryContext rowcontext; /* per-row evaluation context */ uint64 bytes_processed; /* number of bytes processed so far */ } CopyToStateData; @@ -126,12 +117,9 @@ static const char BinarySignature[11] = "PGCOPY\n\377\r\n\0"; static void EndCopy(CopyToState cstate); static void ClosePipeToProgram(CopyToState cstate); static void CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot); - -/* Callbacks for copy_attribute_out */ -static void CopyAttributeOutText(CopyToState cstate, const char *string, - bool use_quote); +static void CopyAttributeOutText(CopyToState cstate, const char *string); static void CopyAttributeOutCSV(CopyToState cstate, const char *string, - bool use_quote); + bool use_quote, bool single_attr); /* Low-level communications functions */ static void SendCopyBegin(CopyToState cstate); @@ -445,15 +433,6 @@ BeginCopyTo(ParseState *pstate, /* Extract options from the statement node tree */ ProcessCopyOptions(pstate, &cstate->opts, false /* is_from */ , options); - /* Set output representation callback */ - if (!cstate->opts.binary) - { - if (cstate->opts.csv_mode) - cstate->copy_attribute_out = CopyAttributeOutCSV; - else - cstate->copy_attribute_out = CopyAttributeOutText; - } - /* Process the source/target relation or query */ if (rel) { @@ -857,8 +836,11 @@ DoCopyTo(CopyToState cstate) colname = NameStr(TupleDescAttr(tupDesc, attnum - 1)->attname); - /* Ignore quotes */ - cstate->copy_attribute_out(cstate, colname, false); + if (cstate->opts.csv_mode) + CopyAttributeOutCSV(cstate, colname, false, + list_length(cstate->attnumlist) == 1); + else + CopyAttributeOutText(cstate, colname); } CopySendEndOfRow(cstate); @@ -968,9 +950,12 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) { string = OutputFunctionCall(&out_functions[attnum - 1], value); - - cstate->copy_attribute_out(cstate, string, - cstate->opts.force_quote_flags[attnum - 1]); + if (cstate->opts.csv_mode) + CopyAttributeOutCSV(cstate, string, + cstate->opts.force_quote_flags[attnum - 1], + list_length(cstate->attnumlist) == 1); + else + CopyAttributeOutText(cstate, string); } else { @@ -1000,8 +985,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) } while (0) static void -CopyAttributeOutText(CopyToState cstate, const char *string, - bool use_quote) +CopyAttributeOutText(CopyToState cstate, const char *string) { const char *ptr; const char *start; @@ -1155,7 +1139,7 @@ CopyAttributeOutText(CopyToState cstate, const char *string, */ static void CopyAttributeOutCSV(CopyToState cstate, const char *string, - bool use_quote) + bool use_quote, bool single_attr) { const char *ptr; const char *start; @@ -1163,7 +1147,6 @@ CopyAttributeOutCSV(CopyToState cstate, const char *string, char delimc = cstate->opts.delim[0]; char quotec = cstate->opts.quote[0]; char escapec = cstate->opts.escape[0]; - bool single_attr = (list_length(cstate->attnumlist) == 1); /* force quoting if it matches null_print (before conversion!) */ if (!use_quote && strcmp(string, cstate->opts.null_print) == 0)