From: Sutou Kouhei <kou@clear-code.com>
To: sawada.mshk@gmail.com
Cc: michael@paquier.xyz
Cc: pgsql-hackers@postgresql.org
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Date: Thu, 21 Nov 2024 11:55:31 +0900 (JST)
Message-ID: <20241121.115531.1600295431613712107.kou@clear-code.com> (raw)
In-Reply-To: <CAD21AoA1s0nzjGU9t3N_uNdg3SZeOxXyH3rQfxYFEN3Y7JrKRQ@mail.gmail.com>
References: <20241119.103115.418773618386970482.kou@clear-code.com>
<CAD21AoByMxtzg9TExQCm2jm1oCN1DwFtNj7Cf_ewSyAwV1c7iQ@mail.gmail.com>
<CAD21AoA1s0nzjGU9t3N_uNdg3SZeOxXyH3rQfxYFEN3Y7JrKRQ@mail.gmail.com>
Hi,
In <CAD21AoA1s0nzjGU9t3N_uNdg3SZeOxXyH3rQfxYFEN3Y7JrKRQ@mail.gmail.com>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Wed, 20 Nov 2024 14:14:27 -0800,
Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I've extracted the changes to refactor COPY TO/FROM to use the format
> callback routines from v23 patch set, which seems to be a better patch
> split to me. Also, I've reviewed these changes and made some changes
> on top of them. The attached patches are:
>
> 0001: make COPY TO use CopyToRoutine.
> 0002: minor changes to 0001 patch. will be fixed up.
> 0003: make COPY FROM use CopyFromRoutine.
> 0004: minor changes to 0003 patch. will be fixed up.
>
> I've confirmed that v24 has a similar performance improvement to v23.
> Please check these extractions and minor change suggestions.
Thanks. Here are my comments:
0002:
+/* TEXT format */
"text" may be better than "TEXT". We use "text" not "TEXT"
in other places.
+static const CopyToRoutine CopyToRoutineText = {
+ .CopyToStart = CopyToTextLikeStart,
+ .CopyToOutFunc = CopyToTextLikeOutFunc,
+ .CopyToOneRow = CopyToTextOneRow,
+ .CopyToEnd = CopyToTextLikeEnd,
+};
+/* BINARY format */
"binary" may be better than "BINARY". We use "binary" not
"BINARY" in other places.
+static const CopyToRoutine CopyToRoutineBinary = {
+ .CopyToStart = CopyToBinaryStart,
+ .CopyToOutFunc = CopyToBinaryOutFunc,
+ .CopyToOneRow = CopyToBinaryOneRow,
+ .CopyToEnd = CopyToBinaryEnd,
+};
+/* Return COPY TO routines for the given option */
option ->
options
+static const CopyToRoutine *
+CopyToGetRoutine(CopyFormatOptions opts)
0003:
diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.hindex 99981b1579..224fda172e 100644--- a/src/include/commands/copyapi.h+++ b/src/include/commands/copyapi.h@@ -56,4 +56,46 @@ typedef struct CopyToRoutine
void (*CopyToEnd) (CopyToState cstate);
} CopyToRoutine;
+/*+ * API structure for a COPY FROM format implementation. Note this must be
Should we remove a tab character here?
0004:
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.cindex e77986f9a9..7f1de8a42b 100644--- a/src/backend/commands/copyfrom.c+++ b/src/backend/commands/copyfrom.c@@ -106,31 +106,65 @@ typedef struct CopyMultiInsertInfo
/* non-export function prototypes */
static void ClosePipeFromProgram(CopyFromState cstate);
-
/*
- * CopyFromRoutine implementations for text and CSV.+ * built-in format-specific routines. One-row callbacks are defined in
built-in ->
Built-in
/*
- * CopyFromTextLikeInFunc
- *
- * Assign input function data for a relation's attribute in text/CSV format.
+ * COPY FROM routines for built-in formats.
++
"+" ->
" *"
+/* TEXT format */
TEXT -> text?
+/* BINARY format */
BINARY -> binary?
+/* Return COPY FROM routines for the given option */
option ->
options
+static const CopyFromRoutine *
+CopyFromGetRoutine(CopyFormatOptions opts)
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.cindex 0447c4df7e..5416583e94 100644--- a/src/backend/commands/copyfromparse.c+++ b/src/backend/commands/copyfromparse.c
+static bool CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext,
+ Datum *values, bool *nulls, bool is_csv);
Oh, I didn't know that we don't need inline in a function
declaration.
@@ -1237,7 +1219,7 @@ CopyReadLine(CopyFromState cstate, bool is_csv)
/*
* CopyReadLineText - inner loop of CopyReadLine for text mode
*/
-static pg_attribute_always_inline bool
+static bool
CopyReadLineText(CopyFromState cstate, bool is_csv)
Is this an intentional change?
CopyReadLineText() has "bool in_csv".
Thanks,
--
kou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: kou@clear-code.com, sawada.mshk@gmail.com, michael@paquier.xyz
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
In-Reply-To: <20241121.115531.1600295431613712107.kou@clear-code.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox