public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v7 2/2] Don't proc_exit() in startup's SIGTERM handler if forked by system().
13+ messages / 4 participants
[nested] [flat]
* [PATCH v7 2/2] Don't proc_exit() in startup's SIGTERM handler if forked by system().
@ 2023-02-14 17:44 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Nathan Bossart @ 2023-02-14 17:44 UTC (permalink / raw)
Instead, emit a message to STDERR and _exit() in this case. This
change also adds assertions to proc_exit(), ProcKill(), and
AuxiliaryProcKill() to verify that these functions are not called
by a process forked by system(), etc.
---
src/backend/postmaster/startup.c | 20 +++++++++++++++++++-
src/backend/storage/ipc/ipc.c | 3 +++
src/backend/storage/lmgr/proc.c | 2 ++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index efc2580536..bace915881 100644
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -19,6 +19,8 @@
*/
#include "postgres.h"
+#include <unistd.h>
+
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
@@ -121,7 +123,23 @@ StartupProcShutdownHandler(SIGNAL_ARGS)
int save_errno = errno;
if (in_restore_command)
- proc_exit(1);
+ {
+ /*
+ * If we are in a child process (e.g., forked by system() in
+ * RestoreArchivedFile()), we don't want to call any exit callbacks.
+ * The parent will take care of that.
+ */
+ if (MyProcPid == (int) getpid())
+ proc_exit(1);
+ else
+ {
+ const char msg[] = "StartupProcShutdownHandler() called in child process\n";
+ int rc pg_attribute_unused();
+
+ rc = write(STDERR_FILENO, msg, sizeof(msg));
+ _exit(1);
+ }
+ }
else
shutdown_requested = true;
WakeupRecovery();
diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index 1904d21795..d5097dc008 100644
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -103,6 +103,9 @@ static int on_proc_exit_index,
void
proc_exit(int code)
{
+ /* proc_exit() is not safe in forked processes from system(), etc. */
+ Assert(MyProcPid == (int) getpid());
+
/* Clean up everything that must be cleaned up */
proc_exit_prepare(code);
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 22b4278610..e3da0622d7 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -805,6 +805,7 @@ ProcKill(int code, Datum arg)
dlist_head *procgloballist;
Assert(MyProc != NULL);
+ Assert(MyProcPid == (int) getpid()); /* not safe if forked by system(), etc. */
/* Make sure we're out of the sync rep lists */
SyncRepCleanupAtProcExit();
@@ -925,6 +926,7 @@ AuxiliaryProcKill(int code, Datum arg)
PGPROC *proc;
Assert(proctype >= 0 && proctype < NUM_AUXILIARY_PROCS);
+ Assert(MyProcPid == (int) getpid()); /* not safe if forked by system(), etc. */
auxproc = &AuxiliaryProcs[proctype];
--
2.25.1
--u3/rZRmxL6MmkK24--
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
@ 2024-02-07 04:33 Michael Paquier <[email protected]>
2024-02-09 04:19 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
0 siblings, 2 replies; 13+ messages in thread
From: Michael Paquier @ 2024-02-07 04:33 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Sutou Kouhei <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
On Tue, Feb 06, 2024 at 03:33:36PM -0800, Andres Freund wrote:
> Well, you can't just do that, because there's only one caller, namely
> CopyToTextOneRow(). What I am trying to suggest is something like the
> attached, just a quick hacky POC. Namely to split out CSV support from
> CopyToTextOneRow() by introducing CopyToCSVOneRow(), and to avoid code
> duplication by moving the code into a new CopyToTextLikeOneRow().
Ah, OK. Got it now.
> I named it CopyToTextLike* here, because it seems confusing that some Text*
> are used for both CSV and text and others are actually just for text. But if
> were to go for that, we should go further.
This can always be argued later.
> To test the performnce effects I chose to remove the pointless encoding
> "check" we're discussing in the other thread, as it makes it harder to see the
> time differences due to the per-attribute code. I did three runs of pgbench
> -t of [1] and chose the fastest result for each.
>
> With turbo mode and power saving disabled:
> Avg Time
> HEAD 995.349
> Remove Encoding Check 870.793
> v13-0001 869.678
> Remove out callback 839.508
Hmm. That explains why I was not seeing any differences with this
callback then. It seems to me that the order of actions to take is
clear, like:
- Revert 2889fd23be56 to keep a clean state of the tree, now done with
1aa8324b81fa.
- Dive into the strlen() issue, as it really looks like this can
create more simplifications for the patch discussed on this thread
with COPY TO.
- 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.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
@ 2024-02-09 04:19 ` Sutou Kouhei <[email protected]>
2024-02-09 04:40 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
1 sibling, 1 reply; 13+ messages in thread
From: Sutou Kouhei @ 2024-02-09 04:19 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
Hi,
In <[email protected]>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Wed, 7 Feb 2024 13:33:18 +0900,
Michael Paquier <[email protected]> wrote:
> Hmm. That explains why I was not seeing any differences with this
> callback then. It seems to me that the order of actions to take is
> clear, like:
> - Revert 2889fd23be56 to keep a clean state of the tree, now done with
> 1aa8324b81fa.
Done.
> - Dive into the strlen() issue, as it really looks like this can
> create more simplifications for the patch discussed on this thread
> with COPY TO.
Done: b619852086ed2b5df76631f5678f60d3bebd3745
> - 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.
Are you already working on this? Do you want me to write the
next patch based on the current master?
Thanks,
--
kou
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:19 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
@ 2024-02-09 04:40 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Michael Paquier @ 2024-02-09 04:40 UTC (permalink / raw)
To: Sutou Kouhei <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
On Fri, Feb 09, 2024 at 01:19:50PM +0900, Sutou Kouhei wrote:
> Are you already working on this? Do you want me to write the
> next patch based on the current master?
No need for a new patch, thanks. I've spent some time today doing a
rebase and measuring the whole, without seeing a degradation with what
should be the worst cases for COPY TO and FROM:
https://www.postgresql.org/message-id/ZcWoTr1N0GELFA9E%40paquier.xyz
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
@ 2024-02-09 04:21 ` Michael Paquier <[email protected]>
2024-02-09 07:32 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
2024-02-09 19:27 ` Re: Make COPY format extendable: Extract COPY TO format implementations Andres Freund <[email protected]>
1 sibling, 2 replies; 13+ messages in thread
From: Michael Paquier @ 2024-02-09 04:21 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Sutou Kouhei <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
On Wed, Feb 07, 2024 at 01:33:18PM +0900, Michael Paquier wrote:
> Hmm. That explains why I was not seeing any differences with this
> callback then. It seems to me that the order of actions to take is
> clear, like:
> - Revert 2889fd23be56 to keep a clean state of the tree, now done with
> 1aa8324b81fa.
> - Dive into the strlen() issue, as it really looks like this can
> create more simplifications for the patch discussed on this thread
> with COPY TO.
This has been done this morning with b619852086ed.
> - 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. While on it, I've
looked at more profiles and did more runtime checks.
Some runtimes, in (ms), average of 15 runs, 30 int attributes on 5M
rows as mentioned above:
COPY FROM text binary
HEAD 6066 7110
v14 6087 7105
COPY TO text binary
HEAD 6591 10161
v14 6508 10189
And here are some profiles, where I'm not seeing an impact at
row-level with the addition of the callbacks:
COPY FROM, text, master:
- 66.59% 16.10% postgres postgres [.] NextCopyFrom ▒ - 50.50% NextCopyFrom
- 30.75% NextCopyFromRawFields
+ 15.93% CopyReadLine
13.73% CopyReadAttributesText
- 19.43% InputFunctionCallSafe
+ 13.49% int4in
0.77% pg_strtoint32_safe
+ 16.10% _start
COPY FROM, text, v14:
- 66.42% 0.74% postgres postgres [.] NextCopyFrom
- 65.67% NextCopyFrom
- 65.51% CopyFromTextOneRow
- 30.25% NextCopyFromRawFields
+ 16.14% CopyReadLine
13.40% CopyReadAttributesText
- 18.96% InputFunctionCallSafe
+ 13.15% int4in
0.70% pg_strtoint32_safe
+ 0.74% _start
COPY TO, binary, master
- 90.32% 7.14% postgres postgres [.] CopyOneRowTo
- 83.18% CopyOneRowTo
+ 60.30% SendFunctionCall
+ 10.99% appendBinaryStringInfo
+ 3.67% MemoryContextReset
+ 2.89% CopySendEndOfRow
0.89% memcpy@plt
0.66% 0xffffa052db5c
0.62% enlargeStringInfo
0.56% pgstat_progress_update_param
+ 7.14% _start
COPY TO, binary, v14
- 90.96% 0.21% postgres postgres [.] CopyOneRowTo
- 90.75% CopyOneRowTo
- 81.86% CopyToBinaryOneRow
+ 59.17% SendFunctionCall
+ 10.56% appendBinaryStringInfo
1.10% enlargeStringInfo
0.59% int4send
0.57% memcpy@plt
+ 3.68% MemoryContextReset
+ 2.83% CopySendEndOfRow
1.13% appendBinaryStringInfo
0.58% SendFunctionCall
0.58% pgstat_progress_update_param
Are there any comments about this v14? Sutou-san?
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. 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.
--
Michael
Attachments:
[text/x-diff] v14-0001-Extract-COPY-FROM-TO-format-implementations.patch (36.1K, ../../[email protected]/2-v14-0001-Extract-COPY-FROM-TO-format-implementations.patch)
download | inline diff:
From 6aa7f8cb65e792def8dda631fed19404db0a88af Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Tue, 6 Feb 2024 08:40:17 +0900
Subject: [PATCH v14] Extract COPY FROM/TO format implementations
This doesn't change the current behavior. This just introduces a set of
copy routines called CopyFromRoutine and CopyToRoutine, which just has
function pointers of format implementation like TupleTableSlotOps, and
use it for existing "text", "csv" and "binary" format implementations.
There are plans to extend that more in the future with custom formats.
This improves performance when a relation has many attributes as this
eliminates format-based if branches. The more the attributes, the
better the performance gain. Blah add some numbers.
---
src/include/commands/copyapi.h | 100 ++++++
src/include/commands/copyfrom_internal.h | 10 +
src/backend/commands/copyfrom.c | 209 ++++++++---
src/backend/commands/copyfromparse.c | 362 ++++++++++---------
src/backend/commands/copyto.c | 434 ++++++++++++++++-------
src/tools/pgindent/typedefs.list | 2 +
6 files changed, 770 insertions(+), 347 deletions(-)
create mode 100644 src/include/commands/copyapi.h
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h
new file mode 100644
index 0000000000..87e0629c2f
--- /dev/null
+++ b/src/include/commands/copyapi.h
@@ -0,0 +1,100 @@
+/*-------------------------------------------------------------------------
+ *
+ * copyapi.h
+ * API for COPY TO/FROM handlers
+ *
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/copyapi.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COPYAPI_H
+#define COPYAPI_H
+
+#include "executor/tuptable.h"
+#include "nodes/execnodes.h"
+
+/* These are private in commands/copy[from|to].c */
+typedef struct CopyFromStateData *CopyFromState;
+typedef struct CopyToStateData *CopyToState;
+
+/*
+ * API structure for a COPY FROM format implementation. Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyFromRoutine
+{
+ /*
+ * Called when COPY FROM is started to set up the input functions
+ * associated to the relation's attributes writing to. `finfo` can be
+ * optionally filled to provide the catalog information of the input
+ * function. `typioparam` can be optionally filled to define the OID of
+ * the type to pass to the input function. `atttypid` is the OID of data
+ * type used by the relation's attribute.
+ */
+ void (*CopyFromInFunc) (CopyFromState cstate, Oid atttypid,
+ FmgrInfo *finfo, Oid *typioparam);
+
+ /*
+ * Called when COPY FROM is started.
+ *
+ * `tupDesc` is the tuple descriptor of the relation where the data needs
+ * to be copied. This can be used for any initialization steps required
+ * by a format.
+ */
+ void (*CopyFromStart) (CopyFromState cstate, TupleDesc tupDesc);
+
+ /*
+ * Copy one row to a set of `values` and `nulls` of size tupDesc->natts.
+ *
+ * 'econtext' is used to evaluate default expression for each column that
+ * is either not read from the file or is using the DEFAULT option of COPY
+ * FROM. It is NULL if no default values are used.
+ *
+ * Returns false if there are no more tuples to copy.
+ */
+ bool (*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext,
+ Datum *values, bool *nulls);
+
+ /* Called when COPY FROM has ended. */
+ void (*CopyFromEnd) (CopyFromState cstate);
+} CopyFromRoutine;
+
+/*
+ * API structure for a COPY TO format implementation. Note this must be
+ * allocated in a server-lifetime manner, typically as a static const struct.
+ */
+typedef struct CopyToRoutine
+{
+ /*
+ * Called when COPY TO is started to set up the output functions
+ * associated to the relation's attributes reading from. `finfo` can be
+ * optionally filled. `atttypid` is the OID of data type used by the
+ * relation's attribute.
+ */
+ void (*CopyToOutFunc) (CopyToState cstate, Oid atttypid,
+ FmgrInfo *finfo);
+
+ /*
+ * Called when COPY TO is started.
+ *
+ * `tupDesc` is the tuple descriptor of the relation from where the data
+ * is read.
+ */
+ void (*CopyToStart) (CopyToState cstate, TupleDesc tupDesc);
+
+ /*
+ * Copy one row for COPY TO.
+ *
+ * `slot` is the tuple slot where the data is emitted.
+ */
+ void (*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot);
+
+ /* Called when COPY TO has ended */
+ void (*CopyToEnd) (CopyToState cstate);
+} CopyToRoutine;
+
+#endif /* COPYAPI_H */
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index 759f8e3d09..5fb52dc629 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -15,6 +15,7 @@
#define COPYFROM_INTERNAL_H
#include "commands/copy.h"
+#include "commands/copyapi.h"
#include "commands/trigger.h"
#include "nodes/miscnodes.h"
@@ -65,6 +66,9 @@ typedef int (*CopyReadAttributes) (CopyFromState cstate);
*/
typedef struct CopyFromStateData
{
+ /* format routine */
+ const CopyFromRoutine *routine;
+
/* low-level state data */
CopySource copy_src; /* type of copy source */
FILE *copy_file; /* used if copy_src == COPY_FILE */
@@ -200,4 +204,10 @@ extern void ReceiveCopyBinaryHeader(CopyFromState cstate);
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 CopyFromBinaryOneRow(CopyFromState cstate, ExprContext *econtext,
+ Datum *values, bool *nulls);
+
#endif /* COPYFROM_INTERNAL_H */
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 41f6bc43e4..a90b7189b5 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -108,6 +108,160 @@ static char *limit_printout_length(const char *str);
static void ClosePipeFromProgram(CopyFromState cstate);
+
+/*
+ * CopyFromRoutine implementations for text and CSV.
+ */
+
+/*
+ * CopyFromTextInFunc
+ *
+ * Assign input function data for a relation's attribute in text/CSV format.
+ */
+static void
+CopyFromTextInFunc(CopyFromState cstate, Oid atttypid,
+ FmgrInfo *finfo, Oid *typioparam)
+{
+ Oid func_oid;
+
+ getTypeInputInfo(atttypid, &func_oid, typioparam);
+ fmgr_info(func_oid, finfo);
+}
+
+/*
+ * CopyFromTextStart
+ *
+ * Start of COPY FROM for text/CSV format.
+ */
+static void
+CopyFromTextStart(CopyFromState cstate, TupleDesc tupDesc)
+{
+ AttrNumber attr_count;
+
+ /*
+ * If encoding conversion is needed, we need another buffer to hold the
+ * converted input data. Otherwise, we can just point input_buf to the
+ * same buffer as raw_buf.
+ */
+ if (cstate->need_transcoding)
+ {
+ cstate->input_buf = (char *) palloc(INPUT_BUF_SIZE + 1);
+ cstate->input_buf_index = cstate->input_buf_len = 0;
+ }
+ else
+ cstate->input_buf = cstate->raw_buf;
+ cstate->input_reached_eof = false;
+
+ initStringInfo(&cstate->line_buf);
+
+ /* create workspace for CopyReadAttributes results */
+ 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;
+}
+
+/*
+ * CopyFromTextEnd
+ *
+ * End of COPY FROM for text/CSV format.
+ */
+static void
+CopyFromTextEnd(CopyFromState cstate)
+{
+ /* nothing to do */
+}
+
+/*
+ * CopyFromRoutine implementation for "binary".
+ */
+
+/*
+ * CopyFromBinaryInFunc
+ *
+ * Assign input function data for a relation's attribute in binary format.
+ */
+static void
+CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid,
+ FmgrInfo *finfo, Oid *typioparam)
+{
+ Oid func_oid;
+
+ getTypeBinaryInputInfo(atttypid, &func_oid, typioparam);
+ fmgr_info(func_oid, finfo);
+}
+
+/*
+ * CopyFromBinaryStart
+ *
+ * Start of COPY FROM for binary format.
+ */
+static void
+CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc)
+{
+ /* Read and verify binary header */
+ ReceiveCopyBinaryHeader(cstate);
+}
+
+/*
+ * CopyFromBinaryEnd
+ *
+ * End of COPY FROM for binary format.
+ */
+static void
+CopyFromBinaryEnd(CopyFromState cstate)
+{
+ /* nothing to do */
+}
+
+/*
+ * Routines assigned to each format.
++
+ * CSV and text share the same implementation, at the exception of the
+ * copy_read_attributes callback.
+ */
+static const CopyFromRoutine CopyFromRoutineText = {
+ .CopyFromInFunc = CopyFromTextInFunc,
+ .CopyFromStart = CopyFromTextStart,
+ .CopyFromOneRow = CopyFromTextOneRow,
+ .CopyFromEnd = CopyFromTextEnd,
+};
+
+static const CopyFromRoutine CopyFromRoutineCSV = {
+ .CopyFromInFunc = CopyFromTextInFunc,
+ .CopyFromStart = CopyFromTextStart,
+ .CopyFromOneRow = CopyFromTextOneRow,
+ .CopyFromEnd = CopyFromTextEnd,
+};
+
+static const CopyFromRoutine CopyFromRoutineBinary = {
+ .CopyFromInFunc = CopyFromBinaryInFunc,
+ .CopyFromStart = CopyFromBinaryStart,
+ .CopyFromOneRow = CopyFromBinaryOneRow,
+ .CopyFromEnd = CopyFromBinaryEnd,
+};
+
+/*
+ * Define the COPY FROM routines to use for a format.
+ */
+static const CopyFromRoutine *
+CopyFromGetRoutine(CopyFormatOptions opts)
+{
+ if (opts.csv_mode)
+ return &CopyFromRoutineCSV;
+ else if (opts.binary)
+ return &CopyFromRoutineBinary;
+
+ /* default is text */
+ return &CopyFromRoutineText;
+}
+
+
/*
* error context callback for COPY FROM
*
@@ -1386,7 +1540,6 @@ BeginCopyFrom(ParseState *pstate,
num_defaults;
FmgrInfo *in_functions;
Oid *typioparams;
- Oid in_func_oid;
int *defmap;
ExprState **defexprs;
MemoryContext oldcontext;
@@ -1418,6 +1571,9 @@ BeginCopyFrom(ParseState *pstate,
/* Extract options from the statement node tree */
ProcessCopyOptions(pstate, &cstate->opts, true /* is_from */ , options);
+ /* Set format routine */
+ cstate->routine = CopyFromGetRoutine(cstate->opts);
+
/* Process the target relation */
cstate->rel = rel;
@@ -1571,25 +1727,6 @@ BeginCopyFrom(ParseState *pstate,
cstate->raw_buf_index = cstate->raw_buf_len = 0;
cstate->raw_reached_eof = false;
- if (!cstate->opts.binary)
- {
- /*
- * If encoding conversion is needed, we need another buffer to hold
- * the converted input data. Otherwise, we can just point input_buf
- * to the same buffer as raw_buf.
- */
- if (cstate->need_transcoding)
- {
- cstate->input_buf = (char *) palloc(INPUT_BUF_SIZE + 1);
- cstate->input_buf_index = cstate->input_buf_len = 0;
- }
- else
- cstate->input_buf = cstate->raw_buf;
- cstate->input_reached_eof = false;
-
- initStringInfo(&cstate->line_buf);
- }
-
initStringInfo(&cstate->attribute_buf);
/* Assign range table and rteperminfos, we'll need them in CopyFrom. */
@@ -1622,13 +1759,9 @@ BeginCopyFrom(ParseState *pstate,
continue;
/* Fetch the input function and typioparam info */
- if (cstate->opts.binary)
- getTypeBinaryInputInfo(att->atttypid,
- &in_func_oid, &typioparams[attnum - 1]);
- else
- getTypeInputInfo(att->atttypid,
- &in_func_oid, &typioparams[attnum - 1]);
- fmgr_info(in_func_oid, &in_functions[attnum - 1]);
+ cstate->routine->CopyFromInFunc(cstate, att->atttypid,
+ &in_functions[attnum - 1],
+ &typioparams[attnum - 1]);
/* Get default info if available */
defexprs[attnum - 1] = NULL;
@@ -1763,25 +1896,7 @@ BeginCopyFrom(ParseState *pstate,
pgstat_progress_update_multi_param(3, progress_cols, progress_vals);
- if (cstate->opts.binary)
- {
- /* Read and verify binary header */
- ReceiveCopyBinaryHeader(cstate);
- }
-
- /* create workspace for CopyReadAttributes results */
- if (!cstate->opts.binary)
- {
- AttrNumber attr_count = list_length(cstate->attnumlist);
-
- cstate->max_fields = attr_count;
- cstate->raw_fields = (char **) palloc(attr_count * sizeof(char *));
-
- if (cstate->opts.csv_mode)
- cstate->copy_read_attributes = CopyReadAttributesCSV;
- else
- cstate->copy_read_attributes = CopyReadAttributesText;
- }
+ cstate->routine->CopyFromStart(cstate, tupDesc);
MemoryContextSwitchTo(oldcontext);
@@ -1794,6 +1909,8 @@ BeginCopyFrom(ParseState *pstate,
void
EndCopyFrom(CopyFromState cstate)
{
+ cstate->routine->CopyFromEnd(cstate);
+
/* No COPY FROM related resources except memory. */
if (cstate->is_program)
{
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 906756362e..c45f9ae134 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -832,6 +832,200 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
return true;
}
+/*
+ * CopyFromTextOneRow
+ *
+ * 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)
+{
+ TupleDesc tupDesc;
+ AttrNumber attr_count;
+ FmgrInfo *in_functions = cstate->in_functions;
+ Oid *typioparams = cstate->typioparams;
+ ExprState **defexprs = cstate->defexprs;
+ char **field_strings;
+ ListCell *cur;
+ int fldct;
+ int fieldno;
+ char *string;
+
+ tupDesc = RelationGetDescr(cstate->rel);
+ attr_count = list_length(cstate->attnumlist);
+
+ /* read raw fields in the next line */
+ if (!NextCopyFromRawFields(cstate, &field_strings, &fldct))
+ return false;
+
+ /* check for overflowing fields */
+ if (attr_count > 0 && fldct > attr_count)
+ ereport(ERROR,
+ (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+ errmsg("extra data after last expected column")));
+
+ fieldno = 0;
+
+ /* Loop to read the user attributes on the line. */
+ foreach(cur, cstate->attnumlist)
+ {
+ int attnum = lfirst_int(cur);
+ int m = attnum - 1;
+ Form_pg_attribute att = TupleDescAttr(tupDesc, m);
+
+ if (fieldno >= fldct)
+ ereport(ERROR,
+ (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+ errmsg("missing data for column \"%s\"",
+ NameStr(att->attname))));
+ string = field_strings[fieldno++];
+
+ if (cstate->convert_select_flags &&
+ !cstate->convert_select_flags[m])
+ {
+ /* ignore input field, leaving column as NULL */
+ continue;
+ }
+
+ cstate->cur_attname = NameStr(att->attname);
+ cstate->cur_attval = string;
+
+ if (cstate->opts.csv_mode)
+ {
+ if (string == NULL &&
+ cstate->opts.force_notnull_flags[m])
+ {
+ /*
+ * FORCE_NOT_NULL option is set and column is NULL - convert
+ * it to the NULL string.
+ */
+ string = cstate->opts.null_print;
+ }
+ else if (string != NULL && cstate->opts.force_null_flags[m]
+ && strcmp(string, cstate->opts.null_print) == 0)
+ {
+ /*
+ * FORCE_NULL option is set and column matches the NULL
+ * string. It must have been quoted, or otherwise the string
+ * would already have been set to NULL. Convert it to NULL as
+ * specified.
+ */
+ string = NULL;
+ }
+ }
+
+ if (string != NULL)
+ nulls[m] = false;
+
+ if (cstate->defaults[m])
+ {
+ /*
+ * The caller must supply econtext and have switched into the
+ * per-tuple memory context in it.
+ */
+ Assert(econtext != NULL);
+ Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory);
+
+ 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]))
+ {
+ cstate->num_errors++;
+ return true;
+ }
+
+ cstate->cur_attname = NULL;
+ cstate->cur_attval = NULL;
+ }
+
+ Assert(fieldno == attr_count);
+
+ return true;
+}
+
+/*
+ * CopyFromBinaryOneRow
+ *
+ * Copy one row to a set of `values` and `nulls` for the binary format.
+ */
+bool
+CopyFromBinaryOneRow(CopyFromState cstate, ExprContext *econtext,
+ Datum *values, bool *nulls)
+{
+ TupleDesc tupDesc;
+ AttrNumber attr_count;
+ FmgrInfo *in_functions = cstate->in_functions;
+ Oid *typioparams = cstate->typioparams;
+ int16 fld_count;
+ ListCell *cur;
+
+ tupDesc = RelationGetDescr(cstate->rel);
+ attr_count = list_length(cstate->attnumlist);
+
+ cstate->cur_lineno++;
+
+ if (!CopyGetInt16(cstate, &fld_count))
+ {
+ /* EOF detected (end of file, or protocol-level EOF) */
+ return false;
+ }
+
+ if (fld_count == -1)
+ {
+ /*
+ * Received EOF marker. Wait for the protocol-level EOF, and complain
+ * if it doesn't come immediately. In COPY FROM STDIN, this ensures
+ * that we correctly handle CopyFail, if client chooses to send that
+ * now. When copying from file, we could ignore the rest of the file
+ * like in text mode, but we choose to be consistent with the COPY
+ * FROM STDIN case.
+ */
+ char dummy;
+
+ if (CopyReadBinaryData(cstate, &dummy, 1) > 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+ errmsg("received copy data after EOF marker")));
+ return false;
+ }
+
+ if (fld_count != attr_count)
+ ereport(ERROR,
+ (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+ errmsg("row field count is %d, expected %d",
+ (int) fld_count, attr_count)));
+
+ foreach(cur, cstate->attnumlist)
+ {
+ int attnum = lfirst_int(cur);
+ int m = attnum - 1;
+ Form_pg_attribute att = TupleDescAttr(tupDesc, m);
+
+ cstate->cur_attname = NameStr(att->attname);
+ values[m] = CopyReadBinaryAttribute(cstate,
+ &in_functions[m],
+ typioparams[m],
+ att->atttypmod,
+ &nulls[m]);
+ cstate->cur_attname = NULL;
+ }
+
+ return true;
+}
+
/*
* Read next tuple from file for COPY FROM. Return false if no more tuples.
*
@@ -841,7 +1035,8 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
* read from the file, and DEFAULT option is unset.
*
* 'values' and 'nulls' arrays must be the same length as columns of the
- * relation passed to BeginCopyFrom. This function fills the arrays.
+ * relation passed to BeginCopyFrom. This function calls the format routine
+ * that should fill the arrays.
*/
bool
NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
@@ -849,181 +1044,22 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
{
TupleDesc tupDesc;
AttrNumber num_phys_attrs,
- attr_count,
num_defaults = cstate->num_defaults;
- FmgrInfo *in_functions = cstate->in_functions;
- Oid *typioparams = cstate->typioparams;
int i;
int *defmap = cstate->defmap;
ExprState **defexprs = cstate->defexprs;
tupDesc = RelationGetDescr(cstate->rel);
num_phys_attrs = tupDesc->natts;
- attr_count = list_length(cstate->attnumlist);
/* Initialize all values for row to NULL */
MemSet(values, 0, num_phys_attrs * sizeof(Datum));
MemSet(nulls, true, num_phys_attrs * sizeof(bool));
MemSet(cstate->defaults, false, num_phys_attrs * sizeof(bool));
- if (!cstate->opts.binary)
- {
- char **field_strings;
- ListCell *cur;
- int fldct;
- int fieldno;
- char *string;
-
- /* read raw fields in the next line */
- if (!NextCopyFromRawFields(cstate, &field_strings, &fldct))
- return false;
-
- /* check for overflowing fields */
- if (attr_count > 0 && fldct > attr_count)
- ereport(ERROR,
- (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
- errmsg("extra data after last expected column")));
-
- fieldno = 0;
-
- /* Loop to read the user attributes on the line. */
- foreach(cur, cstate->attnumlist)
- {
- int attnum = lfirst_int(cur);
- int m = attnum - 1;
- Form_pg_attribute att = TupleDescAttr(tupDesc, m);
-
- if (fieldno >= fldct)
- ereport(ERROR,
- (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
- errmsg("missing data for column \"%s\"",
- NameStr(att->attname))));
- string = field_strings[fieldno++];
-
- if (cstate->convert_select_flags &&
- !cstate->convert_select_flags[m])
- {
- /* ignore input field, leaving column as NULL */
- continue;
- }
-
- if (cstate->opts.csv_mode)
- {
- if (string == NULL &&
- cstate->opts.force_notnull_flags[m])
- {
- /*
- * FORCE_NOT_NULL option is set and column is NULL -
- * convert it to the NULL string.
- */
- string = cstate->opts.null_print;
- }
- else if (string != NULL && cstate->opts.force_null_flags[m]
- && strcmp(string, cstate->opts.null_print) == 0)
- {
- /*
- * FORCE_NULL option is set and column matches the NULL
- * string. It must have been quoted, or otherwise the
- * string would already have been set to NULL. Convert it
- * to NULL as specified.
- */
- string = NULL;
- }
- }
-
- cstate->cur_attname = NameStr(att->attname);
- cstate->cur_attval = string;
-
- if (string != NULL)
- nulls[m] = false;
-
- if (cstate->defaults[m])
- {
- /*
- * The caller must supply econtext and have switched into the
- * per-tuple memory context in it.
- */
- Assert(econtext != NULL);
- Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory);
-
- 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]))
- {
- cstate->num_errors++;
- return true;
- }
-
- cstate->cur_attname = NULL;
- cstate->cur_attval = NULL;
- }
-
- Assert(fieldno == attr_count);
- }
- else
- {
- /* binary */
- int16 fld_count;
- ListCell *cur;
-
- cstate->cur_lineno++;
-
- if (!CopyGetInt16(cstate, &fld_count))
- {
- /* EOF detected (end of file, or protocol-level EOF) */
- return false;
- }
-
- if (fld_count == -1)
- {
- /*
- * Received EOF marker. Wait for the protocol-level EOF, and
- * complain if it doesn't come immediately. In COPY FROM STDIN,
- * this ensures that we correctly handle CopyFail, if client
- * chooses to send that now. When copying from file, we could
- * ignore the rest of the file like in text mode, but we choose to
- * be consistent with the COPY FROM STDIN case.
- */
- char dummy;
-
- if (CopyReadBinaryData(cstate, &dummy, 1) > 0)
- ereport(ERROR,
- (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
- errmsg("received copy data after EOF marker")));
- return false;
- }
-
- if (fld_count != attr_count)
- ereport(ERROR,
- (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
- errmsg("row field count is %d, expected %d",
- (int) fld_count, attr_count)));
-
- foreach(cur, cstate->attnumlist)
- {
- int attnum = lfirst_int(cur);
- int m = attnum - 1;
- Form_pg_attribute att = TupleDescAttr(tupDesc, m);
-
- cstate->cur_attname = NameStr(att->attname);
- values[m] = CopyReadBinaryAttribute(cstate,
- &in_functions[m],
- typioparams[m],
- att->atttypmod,
- &nulls[m]);
- cstate->cur_attname = NULL;
- }
- }
+ if (!cstate->routine->CopyFromOneRow(cstate, econtext, values,
+ nulls))
+ return false;
/*
* Now compute and insert any defaults available for the columns not
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 20ffc90363..e9f9246532 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -24,6 +24,7 @@
#include "access/xact.h"
#include "access/xlog.h"
#include "commands/copy.h"
+#include "commands/copyapi.h"
#include "commands/progress.h"
#include "executor/execdesc.h"
#include "executor/executor.h"
@@ -71,6 +72,9 @@ typedef enum CopyDest
*/
typedef struct CopyToStateData
{
+ /* format routine */
+ const CopyToRoutine *routine;
+
/* low-level state data */
CopyDest copy_dest; /* type of copy source/destination */
FILE *copy_file; /* used if copy_dest == COPY_FILE */
@@ -131,6 +135,290 @@ static void CopySendEndOfRow(CopyToState cstate);
static void CopySendInt32(CopyToState cstate, int32 val);
static void CopySendInt16(CopyToState cstate, int16 val);
+/*
+ * CopyToRoutine implementations.
+ */
+
+/*
+ * CopyToTextSendEndOfRow
+ *
+ * Apply line terminations for a line sent in text or CSV format depending
+ * on the destination, then send the end of a row.
+ */
+static inline void
+CopyToTextSendEndOfRow(CopyToState cstate)
+{
+ switch (cstate->copy_dest)
+ {
+ case COPY_FILE:
+ /* Default line termination depends on platform */
+#ifndef WIN32
+ CopySendChar(cstate, '\n');
+#else
+ CopySendString(cstate, "\r\n");
+#endif
+ break;
+ case COPY_FRONTEND:
+ /* The FE/BE protocol uses \n as newline for all platforms */
+ CopySendChar(cstate, '\n');
+ break;
+ default:
+ break;
+ }
+
+ /* Now take the actions related to the end of a row */
+ CopySendEndOfRow(cstate);
+}
+
+/*
+ * CopyToTextStart
+ *
+ * Start of COPY TO for text and CSV format.
+ */
+static void
+CopyToTextStart(CopyToState cstate, TupleDesc tupDesc)
+{
+ ListCell *cur;
+
+ /*
+ * For non-binary copy, we need to convert null_print to file encoding,
+ * because it will be sent directly with CopySendString.
+ */
+ if (cstate->need_transcoding)
+ cstate->opts.null_print_client = pg_server_to_any(cstate->opts.null_print,
+ cstate->opts.null_print_len,
+ cstate->file_encoding);
+
+ /* if a header has been requested send the line */
+ if (cstate->opts.header_line)
+ {
+ bool hdr_delim = false;
+
+ foreach(cur, cstate->attnumlist)
+ {
+ int attnum = lfirst_int(cur);
+ char *colname;
+
+ if (hdr_delim)
+ CopySendChar(cstate, cstate->opts.delim[0]);
+ hdr_delim = true;
+
+ colname = NameStr(TupleDescAttr(tupDesc, attnum - 1)->attname);
+
+ if (cstate->opts.csv_mode)
+ CopyAttributeOutCSV(cstate, colname, false);
+ else
+ CopyAttributeOutText(cstate, colname);
+ }
+
+ CopyToTextSendEndOfRow(cstate);
+ }
+}
+
+/*
+ * CopyToTextOutFunc
+ *
+ * Assign output function data for a relation's attribute in text/CSV format.
+ */
+static void
+CopyToTextOutFunc(CopyToState cstate, Oid atttypid, FmgrInfo *finfo)
+{
+ Oid func_oid;
+ bool is_varlena;
+
+ /* Set output function for an attribute */
+ getTypeOutputInfo(atttypid, &func_oid, &is_varlena);
+ fmgr_info(func_oid, finfo);
+}
+
+/*
+ * CopyToTextOneRow
+ *
+ * Process one row for text/CSV format.
+ */
+static void
+CopyToTextOneRow(CopyToState cstate,
+ TupleTableSlot *slot)
+{
+ bool need_delim = false;
+ FmgrInfo *out_functions = cstate->out_functions;
+ ListCell *cur;
+
+ foreach(cur, cstate->attnumlist)
+ {
+ int attnum = lfirst_int(cur);
+ Datum value = slot->tts_values[attnum - 1];
+ bool isnull = slot->tts_isnull[attnum - 1];
+
+ if (need_delim)
+ CopySendChar(cstate, cstate->opts.delim[0]);
+ need_delim = true;
+
+ if (isnull)
+ {
+ CopySendString(cstate, cstate->opts.null_print_client);
+ }
+ else
+ {
+ char *string;
+
+ string = OutputFunctionCall(&out_functions[attnum - 1], value);
+
+ if (cstate->opts.csv_mode)
+ CopyAttributeOutCSV(cstate, string,
+ cstate->opts.force_quote_flags[attnum - 1]);
+ else
+ CopyAttributeOutText(cstate, string);
+ }
+ }
+
+ CopyToTextSendEndOfRow(cstate);
+}
+
+/*
+ * CopyToTextEnd
+ *
+ * End of COPY TO for text/CSV format.
+ */
+static void
+CopyToTextEnd(CopyToState cstate)
+{
+ /* Nothing to do here */
+}
+
+/*
+ * CopyToRoutine implementation for "binary".
+ */
+
+/*
+ * CopyToBinaryStart
+ *
+ * Start of COPY TO for binary format.
+ */
+static void
+CopyToBinaryStart(CopyToState cstate, TupleDesc tupDesc)
+{
+ /* Generate header for a binary copy */
+ int32 tmp;
+
+ /* Signature */
+ CopySendData(cstate, BinarySignature, 11);
+ /* Flags field */
+ tmp = 0;
+ CopySendInt32(cstate, tmp);
+ /* No header extension */
+ tmp = 0;
+ CopySendInt32(cstate, tmp);
+}
+
+/*
+ * CopyToBinaryOutFunc
+ *
+ * Assign output function data for a relation's attribute in binary format.
+ */
+static void
+CopyToBinaryOutFunc(CopyToState cstate, Oid atttypid, FmgrInfo *finfo)
+{
+ Oid func_oid;
+ bool is_varlena;
+
+ /* Set output function for an attribute */
+ getTypeBinaryOutputInfo(atttypid, &func_oid, &is_varlena);
+ fmgr_info(func_oid, finfo);
+}
+
+/*
+ * CopyToBinaryOneRow
+ *
+ * Process one row for binary format.
+ */
+static void
+CopyToBinaryOneRow(CopyToState cstate, TupleTableSlot *slot)
+{
+ FmgrInfo *out_functions = cstate->out_functions;
+ ListCell *cur;
+
+ /* Binary per-tuple header */
+ CopySendInt16(cstate, list_length(cstate->attnumlist));
+
+ foreach(cur, cstate->attnumlist)
+ {
+ int attnum = lfirst_int(cur);
+ Datum value = slot->tts_values[attnum - 1];
+ bool isnull = slot->tts_isnull[attnum - 1];
+
+ if (isnull)
+ {
+ CopySendInt32(cstate, -1);
+ }
+ else
+ {
+ bytea *outputbytes;
+
+ outputbytes = SendFunctionCall(&out_functions[attnum - 1], value);
+ CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
+ CopySendData(cstate, VARDATA(outputbytes),
+ VARSIZE(outputbytes) - VARHDRSZ);
+ }
+ }
+
+ CopySendEndOfRow(cstate);
+}
+
+/*
+ * CopyToBinaryEnd
+ *
+ * End of COPY TO for binary format.
+ */
+static void
+CopyToBinaryEnd(CopyToState cstate)
+{
+ /* Generate trailer for a binary copy */
+ CopySendInt16(cstate, -1);
+ /* Need to flush out the trailer */
+ CopySendEndOfRow(cstate);
+}
+
+/*
+ * CSV and text share the same implementation, at the exception of the
+ * output representation function.
+ */
+static const CopyToRoutine CopyToRoutineText = {
+ .CopyToStart = CopyToTextStart,
+ .CopyToOutFunc = CopyToTextOutFunc,
+ .CopyToOneRow = CopyToTextOneRow,
+ .CopyToEnd = CopyToTextEnd,
+};
+
+static const CopyToRoutine CopyToRoutineCSV = {
+ .CopyToStart = CopyToTextStart,
+ .CopyToOutFunc = CopyToTextOutFunc,
+ .CopyToOneRow = CopyToTextOneRow,
+ .CopyToEnd = CopyToTextEnd,
+};
+
+static const CopyToRoutine CopyToRoutineBinary = {
+ .CopyToStart = CopyToBinaryStart,
+ .CopyToOutFunc = CopyToBinaryOutFunc,
+ .CopyToOneRow = CopyToBinaryOneRow,
+ .CopyToEnd = CopyToBinaryEnd,
+};
+
+/*
+ * Define the COPY TO routines to use for a format. This should be called
+ * after options are parsed.
+ */
+static const CopyToRoutine *
+CopyToGetRoutine(CopyFormatOptions opts)
+{
+ if (opts.csv_mode)
+ return &CopyToRoutineCSV;
+ else if (opts.binary)
+ return &CopyToRoutineBinary;
+
+ /* default is text */
+ return &CopyToRoutineText;
+}
/*
* Send copy start/stop messages for frontend copies. These have changed
@@ -198,16 +486,6 @@ CopySendEndOfRow(CopyToState cstate)
switch (cstate->copy_dest)
{
case COPY_FILE:
- if (!cstate->opts.binary)
- {
- /* Default line termination depends on platform */
-#ifndef WIN32
- CopySendChar(cstate, '\n');
-#else
- CopySendString(cstate, "\r\n");
-#endif
- }
-
if (fwrite(fe_msgbuf->data, fe_msgbuf->len, 1,
cstate->copy_file) != 1 ||
ferror(cstate->copy_file))
@@ -242,10 +520,6 @@ CopySendEndOfRow(CopyToState cstate)
}
break;
case COPY_FRONTEND:
- /* The FE/BE protocol uses \n as newline for all platforms */
- if (!cstate->opts.binary)
- CopySendChar(cstate, '\n');
-
/* Dump the accumulated row as one CopyData message */
(void) pq_putmessage(PqMsg_CopyData, fe_msgbuf->data, fe_msgbuf->len);
break;
@@ -433,6 +707,9 @@ BeginCopyTo(ParseState *pstate,
/* Extract options from the statement node tree */
ProcessCopyOptions(pstate, &cstate->opts, false /* is_from */ , options);
+ /* Set format routine */
+ cstate->routine = CopyToGetRoutine(cstate->opts);
+
/* Process the source/target relation or query */
if (rel)
{
@@ -772,19 +1049,10 @@ DoCopyTo(CopyToState cstate)
foreach(cur, cstate->attnumlist)
{
int attnum = lfirst_int(cur);
- Oid out_func_oid;
- bool isvarlena;
Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
- if (cstate->opts.binary)
- getTypeBinaryOutputInfo(attr->atttypid,
- &out_func_oid,
- &isvarlena);
- else
- getTypeOutputInfo(attr->atttypid,
- &out_func_oid,
- &isvarlena);
- fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]);
+ cstate->routine->CopyToOutFunc(cstate, attr->atttypid,
+ &cstate->out_functions[attnum - 1]);
}
/*
@@ -797,56 +1065,7 @@ DoCopyTo(CopyToState cstate)
"COPY TO",
ALLOCSET_DEFAULT_SIZES);
- if (cstate->opts.binary)
- {
- /* Generate header for a binary copy */
- int32 tmp;
-
- /* Signature */
- CopySendData(cstate, BinarySignature, 11);
- /* Flags field */
- tmp = 0;
- CopySendInt32(cstate, tmp);
- /* No header extension */
- tmp = 0;
- CopySendInt32(cstate, tmp);
- }
- else
- {
- /*
- * For non-binary copy, we need to convert null_print to file
- * encoding, because it will be sent directly with CopySendString.
- */
- if (cstate->need_transcoding)
- cstate->opts.null_print_client = pg_server_to_any(cstate->opts.null_print,
- cstate->opts.null_print_len,
- cstate->file_encoding);
-
- /* if a header has been requested send the line */
- if (cstate->opts.header_line)
- {
- bool hdr_delim = false;
-
- foreach(cur, cstate->attnumlist)
- {
- int attnum = lfirst_int(cur);
- char *colname;
-
- if (hdr_delim)
- CopySendChar(cstate, cstate->opts.delim[0]);
- hdr_delim = true;
-
- colname = NameStr(TupleDescAttr(tupDesc, attnum - 1)->attname);
-
- if (cstate->opts.csv_mode)
- CopyAttributeOutCSV(cstate, colname, false);
- else
- CopyAttributeOutText(cstate, colname);
- }
-
- CopySendEndOfRow(cstate);
- }
- }
+ cstate->routine->CopyToStart(cstate, tupDesc);
if (cstate->rel)
{
@@ -885,13 +1104,7 @@ DoCopyTo(CopyToState cstate)
processed = ((DR_copy *) cstate->queryDesc->dest)->processed;
}
- if (cstate->opts.binary)
- {
- /* Generate trailer for a binary copy */
- CopySendInt16(cstate, -1);
- /* Need to flush out the trailer */
- CopySendEndOfRow(cstate);
- }
+ cstate->routine->CopyToEnd(cstate);
MemoryContextDelete(cstate->rowcontext);
@@ -907,70 +1120,15 @@ DoCopyTo(CopyToState cstate)
static void
CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
{
- bool need_delim = false;
- FmgrInfo *out_functions = cstate->out_functions;
MemoryContext oldcontext;
- ListCell *cur;
- char *string;
MemoryContextReset(cstate->rowcontext);
oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
- if (cstate->opts.binary)
- {
- /* Binary per-tuple header */
- CopySendInt16(cstate, list_length(cstate->attnumlist));
- }
-
/* Make sure the tuple is fully deconstructed */
slot_getallattrs(slot);
- foreach(cur, cstate->attnumlist)
- {
- int attnum = lfirst_int(cur);
- Datum value = slot->tts_values[attnum - 1];
- bool isnull = slot->tts_isnull[attnum - 1];
-
- if (!cstate->opts.binary)
- {
- if (need_delim)
- CopySendChar(cstate, cstate->opts.delim[0]);
- need_delim = true;
- }
-
- if (isnull)
- {
- if (!cstate->opts.binary)
- CopySendString(cstate, cstate->opts.null_print_client);
- else
- CopySendInt32(cstate, -1);
- }
- else
- {
- if (!cstate->opts.binary)
- {
- string = OutputFunctionCall(&out_functions[attnum - 1],
- value);
- if (cstate->opts.csv_mode)
- CopyAttributeOutCSV(cstate, string,
- cstate->opts.force_quote_flags[attnum - 1]);
- else
- CopyAttributeOutText(cstate, string);
- }
- else
- {
- bytea *outputbytes;
-
- outputbytes = SendFunctionCall(&out_functions[attnum - 1],
- value);
- CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ);
- CopySendData(cstate, VARDATA(outputbytes),
- VARSIZE(outputbytes) - VARHDRSZ);
- }
- }
- }
-
- CopySendEndOfRow(cstate);
+ cstate->routine->CopyToOneRow(cstate, slot);
MemoryContextSwitchTo(oldcontext);
}
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 91433d439b..d02a7773e3 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -473,6 +473,7 @@ ConvertRowtypeExpr
CookedConstraint
CopyDest
CopyFormatOptions
+CopyFromRoutine
CopyFromState
CopyFromStateData
CopyHeaderChoice
@@ -482,6 +483,7 @@ CopyMultiInsertInfo
CopyOnErrorChoice
CopySource
CopyStmt
+CopyToRoutine
CopyToState
CopyToStateData
Cost
--
2.43.0
[application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
@ 2024-02-09 07:32 ` Sutou Kouhei <[email protected]>
2024-02-09 08:25 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
1 sibling, 1 reply; 13+ messages in thread
From: Sutou Kouhei @ 2024-02-09 07:32 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
Hi,
In <[email protected]>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 9 Feb 2024 13:21:34 +0900,
Michael Paquier <[email protected]> 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
Attachments:
[text/x-patch] remove-copy-read-attributes.diff (7.8K, ../../[email protected]/2-remove-copy-read-attributes.diff)
download | inline diff:
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);
[text/x-patch] remove-copy-read-attributes-and-optimize-copy-read-line.diff (13.6K, ../../[email protected]/3-remove-copy-read-attributes-and-optimize-copy-read-line.diff)
download | inline diff:
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);
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 07:32 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
@ 2024-02-09 08:25 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Michael Paquier @ 2024-02-09 08:25 UTC (permalink / raw)
To: Sutou Kouhei <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
On Fri, Feb 09, 2024 at 04:32:05PM +0900, Sutou Kouhei wrote:
> In <[email protected]>
> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 9 Feb 2024 13:21:34 +0900,
> Michael Paquier <[email protected]> wrote:
>> 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.
Perhaps, but I'm not sure, TBH. But perhaps others can comment on
this point. This surely needs to be studied closely.
> 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. ...
4. is going to need 3. At this point 3.b sounds like the main thing
to tackle first if we want to get something usable for the end-user
into this release, at least. Still 2 is important for pluggability
as we pass the cstates across all the routines and custom formats want
to save their own data, so this split sounds OK. I am not sure how
much of 3.a we really need to do for the in-core formats.
> 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.
FWIW, using inlining did not show any performance change here.
Perhaps that's only because this is called in the COPY FROM path once
per row (even for the case of using 1 attribute with blackhole_am).
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
@ 2024-02-09 19:27 ` Andres Freund <[email protected]>
2024-02-10 01:02 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-13 08:33 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
1 sibling, 2 replies; 13+ messages in thread
From: Andres Freund @ 2024-02-09 19:27 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Sutou Kouhei <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
Hi,
On 2024-02-09 13:21:34 +0900, Michael Paquier 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.
> +/*
> + * CopyFromTextStart
> + *
> + * Start of COPY FROM for text/CSV format.
> + */
> +static void
> +CopyFromTextStart(CopyFromState cstate, TupleDesc tupDesc)
> +{
> + AttrNumber attr_count;
> +
> + /*
> + * If encoding conversion is needed, we need another buffer to hold the
> + * converted input data. Otherwise, we can just point input_buf to the
> + * same buffer as raw_buf.
> + */
> + if (cstate->need_transcoding)
> + {
> + cstate->input_buf = (char *) palloc(INPUT_BUF_SIZE + 1);
> + cstate->input_buf_index = cstate->input_buf_len = 0;
> + }
> + else
> + cstate->input_buf = cstate->raw_buf;
> + cstate->input_reached_eof = false;
> +
> + initStringInfo(&cstate->line_buf);
Seems kinda odd that we have a supposedly extensible API that then stores all
this stuff in the non-extensible CopyFromState.
> + /* create workspace for CopyReadAttributes results */
> + attr_count = list_length(cstate->attnumlist);
> + cstate->max_fields = attr_count;
Why is this here? This seems like generic code, not text format specific.
> + 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?
And, why is this done here? Shouldn't this decision have been made prior to
even calling CopyFromTextStart()?
> +/*
> + * CopyFromTextOneRow
> + *
> + * Copy one row to a set of `values` and `nulls` for the text and CSV
> + * formats.
> + */
I'm very doubtful it's a good idea to combine text and CSV here. They have
basically no shared parsing code, so what's the point in sending them through
one input routine?
> +bool
> +CopyFromTextOneRow(CopyFromState cstate,
> + ExprContext *econtext,
> + Datum *values,
> + bool *nulls)
> +{
> + TupleDesc tupDesc;
> + AttrNumber attr_count;
> + FmgrInfo *in_functions = cstate->in_functions;
> + Oid *typioparams = cstate->typioparams;
> + ExprState **defexprs = cstate->defexprs;
> + char **field_strings;
> + ListCell *cur;
> + int fldct;
> + int fieldno;
> + char *string;
> +
> + tupDesc = RelationGetDescr(cstate->rel);
> + attr_count = list_length(cstate->attnumlist);
> +
> + /* read raw fields in the next line */
> + if (!NextCopyFromRawFields(cstate, &field_strings, &fldct))
> + return false;
> +
> + /* check for overflowing fields */
> + if (attr_count > 0 && fldct > attr_count)
> + ereport(ERROR,
> + (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
> + errmsg("extra data after last expected column")));
It bothers me that we look to be ending up with different error handling
across the various output formats, particularly if they're ending up in
extensions. That'll make it harder to evolve this code in the future.
> + fieldno = 0;
> +
> + /* Loop to read the user attributes on the line. */
> + foreach(cur, cstate->attnumlist)
> + {
> + int attnum = lfirst_int(cur);
> + int m = attnum - 1;
> + Form_pg_attribute att = TupleDescAttr(tupDesc, m);
> +
> + if (fieldno >= fldct)
> + ereport(ERROR,
> + (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
> + errmsg("missing data for column \"%s\"",
> + NameStr(att->attname))));
> + string = field_strings[fieldno++];
> +
> + if (cstate->convert_select_flags &&
> + !cstate->convert_select_flags[m])
> + {
> + /* ignore input field, leaving column as NULL */
> + continue;
> + }
> +
> + cstate->cur_attname = NameStr(att->attname);
> + cstate->cur_attval = string;
> +
> + if (cstate->opts.csv_mode)
> + {
More unfortunate intermingling of multiple formats in a single routine.
> +
> + if (cstate->defaults[m])
> + {
> + /*
> + * The caller must supply econtext and have switched into the
> + * per-tuple memory context in it.
> + */
> + Assert(econtext != NULL);
> + Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory);
> +
> + values[m] = ExecEvalExpr(defexprs[m], econtext, &nulls[m]);
> + }
I don't think it's good that we end up with this code in different copy
implementations.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 19:27 ` Re: Make COPY format extendable: Extract COPY TO format implementations Andres Freund <[email protected]>
@ 2024-02-10 01:02 ` Michael Paquier <[email protected]>
1 sibling, 0 replies; 13+ messages in thread
From: Michael Paquier @ 2024-02-10 01:02 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Sutou Kouhei <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
On Fri, Feb 09, 2024 at 11:27:05AM -0800, Andres Freund wrote:
> On 2024-02-09 13:21:34 +0900, Michael Paquier 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.
You mean to initialize once its memory and let the internal routines
call InitFunctionCallInfoData for each attribute. Sounds like a good
idea, doing that for HEAD before the main patch. More impact with
more attributes.
>> +/*
>> + * CopyFromTextStart
>> + *
>> + * Start of COPY FROM for text/CSV format.
>> + */
>> +static void
>> +CopyFromTextStart(CopyFromState cstate, TupleDesc tupDesc)
>> +{
>> + AttrNumber attr_count;
>> +
>> + /*
>> + * If encoding conversion is needed, we need another buffer to hold the
>> + * converted input data. Otherwise, we can just point input_buf to the
>> + * same buffer as raw_buf.
>> + */
>> + if (cstate->need_transcoding)
>> + {
>> + cstate->input_buf = (char *) palloc(INPUT_BUF_SIZE + 1);
>> + cstate->input_buf_index = cstate->input_buf_len = 0;
>> + }
>> + else
>> + cstate->input_buf = cstate->raw_buf;
>> + cstate->input_reached_eof = false;
>> +
>> + initStringInfo(&cstate->line_buf);
>
> Seems kinda odd that we have a supposedly extensible API that then stores all
> this stuff in the non-extensible CopyFromState.
That relates to the introduction of the the opaque pointer mentioned
upthread to point to a per-format structure, where we'd store data
specific to each format.
>> + /* create workspace for CopyReadAttributes results */
>> + attr_count = list_length(cstate->attnumlist);
>> + cstate->max_fields = attr_count;
>
> Why is this here? This seems like generic code, not text format specific.
We don't care about that for binary.
>> +/*
>> + * CopyFromTextOneRow
>> + *
>> + * Copy one row to a set of `values` and `nulls` for the text and CSV
>> + * formats.
>> + */
>
> I'm very doubtful it's a good idea to combine text and CSV here. They have
> basically no shared parsing code, so what's the point in sending them through
> one input routine?
The code shared between text and csv involves a path called once per
attribute. TBH, I am not sure how much of the NULL handling should be
put outside the per-row routine as these options are embedded in the
core options. So I don't have a better idea on this one than what's
proposed here if we cannot dispatch the routine calls once per
attribute.
>> + /* read raw fields in the next line */
>> + if (!NextCopyFromRawFields(cstate, &field_strings, &fldct))
>> + return false;
>> +
>> + /* check for overflowing fields */
>> + if (attr_count > 0 && fldct > attr_count)
>> + ereport(ERROR,
>> + (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
>> + errmsg("extra data after last expected column")));
>
> It bothers me that we look to be ending up with different error handling
> across the various output formats, particularly if they're ending up in
> extensions. That'll make it harder to evolve this code in the future.
But different formats may have different requirements, including the
number of attributes detected vs expected. That was not really
nothing me.
>> + if (cstate->opts.csv_mode)
>> + {
>
> More unfortunate intermingling of multiple formats in a single
> routine.
Similar answer as a few paragraphs above. Sutou-san was suggesting to
use an internal routine with fixed arguments instead, which would be
enough at the end with some inline instructions?
>> +
>> + if (cstate->defaults[m])
>> + {
>> + /*
>> + * The caller must supply econtext and have switched into the
>> + * per-tuple memory context in it.
>> + */
>> + Assert(econtext != NULL);
>> + Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory);
>> +
>> + values[m] = ExecEvalExpr(defexprs[m], econtext, &nulls[m]);
>> + }
>
> I don't think it's good that we end up with this code in different copy
> implementations.
Yeah, still we don't care about that for binary.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 19:27 ` Re: Make COPY format extendable: Extract COPY TO format implementations Andres Freund <[email protected]>
@ 2024-02-13 08:33 ` Sutou Kouhei <[email protected]>
2024-02-14 03:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-15 06:51 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
1 sibling, 2 replies; 13+ messages in thread
From: Sutou Kouhei @ 2024-02-13 08:33 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
Hi,
In <[email protected]>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 9 Feb 2024 11:27:05 -0800,
Andres Freund <[email protected]> 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#d...
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
Attachments:
[text/x-patch] prepare-callinfo.diff (8.5K, ../../[email protected]/2-prepare-callinfo.diff)
download | inline diff:
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);
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 19:27 ` Re: Make COPY format extendable: Extract COPY TO format implementations Andres Freund <[email protected]>
2024-02-13 08:33 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
@ 2024-02-14 03:28 ` Michael Paquier <[email protected]>
2024-02-14 05:08 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
1 sibling, 1 reply; 13+ messages in thread
From: Michael Paquier @ 2024-02-14 03:28 UTC (permalink / raw)
To: Sutou Kouhei <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
On Tue, Feb 13, 2024 at 05:33:40PM +0900, Sutou Kouhei wrote:
> Hi,
>
> In <[email protected]>
> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 9 Feb 2024 11:27:05 -0800,
> Andres Freund <[email protected]> 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.
Hmm, I have not studied that much, but my first impression was that we
would not require any new facility in fmgr.c, but perhaps you're right
and it's more elegant to pass a InitFunctionCallInfoData this way.
PrepareInputFunctionCallInfo() looks OK as a name, but I'm less a fan
of PreparedInputFunctionCallSafe() and its "Prepared" part. How about
something like ExecuteInputFunctionCallSafe()?
I may be able to look more at that next week, and I would surely check
the impact of that with a simple COPY query throttled by CPU (more
rows and more attributes the better).
>>> + 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#d...
>
> If it's a desired approach, I can prepare a v15 patch set
> based on the v14 patch set and the approach.
Yes, this one looks like it's using the right angle: we don't rely
anymore in cstate to decide which CopyReadAttributes to use, the
routines do that instead. Note that I've reverted 06bd311bce24 for
the moment, as this is just getting in the way of the main patch, and
that was non-optimal once there is a per-row callback.
> 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();
Perhaps we'd better avoid more callbacks like that, for now.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 19:27 ` Re: Make COPY format extendable: Extract COPY TO format implementations Andres Freund <[email protected]>
2024-02-13 08:33 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
2024-02-14 03:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
@ 2024-02-14 05:08 ` Sutou Kouhei <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Sutou Kouhei @ 2024-02-14 05:08 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
Hi,
In <[email protected]>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Wed, 14 Feb 2024 12:28:38 +0900,
Michael Paquier <[email protected]> wrote:
>> How about the attached patch approach? If it's a desired
>> approach, I can also write a separated patch for COPY TO.
>
> Hmm, I have not studied that much, but my first impression was that we
> would not require any new facility in fmgr.c, but perhaps you're right
> and it's more elegant to pass a InitFunctionCallInfoData this way.
I'm not familiar with the fmgr.c related code base but it
seems that we abstract {,binary-}input function call by
fmgr.c. So I think that it's better that we follow the
design. (If there is a person who knows the fmgr.c related
code base, please help us.)
> PrepareInputFunctionCallInfo() looks OK as a name, but I'm less a fan
> of PreparedInputFunctionCallSafe() and its "Prepared" part. How about
> something like ExecuteInputFunctionCallSafe()?
I understand the feeling. SQL uses "prepared" for "prepared
statement". There are similar function names such as
InputFunctionCall()/InputFunctionCallSafe()/DirectInputFunctionCallSafe(). They
execute (call) an input function but they use "call" not
"execute" for it... So "Execute...Call..." may be
redundant...
How about InputFunctionCallSafeWithInfo(),
InputFunctionCallSafeInfo() or
InputFunctionCallInfoCallSafe()?
> I may be able to look more at that next week, and I would surely check
> the impact of that with a simple COPY query throttled by CPU (more
> rows and more attributes the better).
Thanks!
> Note that I've reverted 06bd311bce24 for
> the moment, as this is just getting in the way of the main patch, and
> that was non-optimal once there is a per-row callback.
Thanks for sharing the information. I'll rebase on master
when I create the v15 patch.
>> 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();
>
> Perhaps we'd better avoid more callbacks like that, for now.
I'll not use a callback for this. I'll not change this part
after we introduce Copy{To,From}Routine. cstate->fcinfo
isn't used some custom COPY format handlers such as Apache
Arrow handler like cstate->in_functions and
cstate->typioparams. But they will be always allocated. It's
a bit wasteful for those handlers but we may not care about
it. So we can always use "if (state->opts.binary)" condition
here.
BTW... This part was wrong... Sorry... It should be:
if (cstate->opts.binary)
cstate->fcinfo = PrepareReceiveFunctionCallInfo();
else
cstate->fcinfo = PrepareInputFunctionCallInfo();
Thanks,
--
kou
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 19:27 ` Re: Make COPY format extendable: Extract COPY TO format implementations Andres Freund <[email protected]>
2024-02-13 08:33 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
@ 2024-02-15 06:51 ` Sutou Kouhei <[email protected]>
1 sibling, 0 replies; 13+ messages in thread
From: Sutou Kouhei @ 2024-02-15 06:51 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers
Hi,
In <[email protected]>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Tue, 13 Feb 2024 17:33:40 +0900 (JST),
Sutou Kouhei <[email protected]> wrote:
> I'll reply other comments later...
I've read other comments and my answers for them are same as
Michael's one.
I'll prepare the v15 patch with static inline functions and
fixed arguments after the fcinfo cache patches are merged. I
think that the v15 patch will be conflicted with fcinfo
cache patches.
Thanks,
--
kou
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2024-02-15 06:51 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14 17:44 [PATCH v7 2/2] Don't proc_exit() in startup's SIGTERM handler if forked by system(). Nathan Bossart <[email protected]>
2024-02-07 04:33 Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:19 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
2024-02-09 04:40 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 04:21 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 07:32 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
2024-02-09 08:25 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-09 19:27 ` Re: Make COPY format extendable: Extract COPY TO format implementations Andres Freund <[email protected]>
2024-02-10 01:02 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-13 08:33 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
2024-02-14 03:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Michael Paquier <[email protected]>
2024-02-14 05:08 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
2024-02-15 06:51 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox