public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v14 1/7] Move extra code out of the Pre/PostRestoreCommand() block. 15+ messages / 3 participants [nested] [flat]
* [PATCH v8 1/2] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index fcc87ff44f..41684418b6 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --FCuugMFkClbJLl1L Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-f.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v10 1/2] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index fcc87ff44f..41684418b6 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --Q68bSM7Ycu6FN28Q Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v13 1/7] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index fcc87ff44f..41684418b6 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --5vNYLRcllDrimb99 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v13-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v15 1/7] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index f3fb92c8f9..b998cd651c 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --k+w/mQv8wyuph6w0 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v15-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v7 1/2] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index fcc87ff44f..41684418b6 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --u3/rZRmxL6MmkK24 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-f.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v9 1/2] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index fcc87ff44f..41684418b6 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --IS0zKkzwUGydFO0o Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-f.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v14 1/7] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index fcc87ff44f..41684418b6 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --IS0zKkzwUGydFO0o Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v14-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v6 1/2] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index fcc87ff44f..41684418b6 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --wac7ysb48OaltWcw Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-f.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v11 1/2] Move extra code out of the Pre/PostRestoreCommand() block. @ 2023-02-23 22:27 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Nathan Bossart @ 2023-02-23 22:27 UTC (permalink / raw) If SIGTERM is received within this block, the startup process will immediately proc_exit() in the signal handler, so it is inadvisable to include any more code than is required in this section. This change moves the code recently added to this block (see 1b06d7b and 7fed801) to outside of the block. This ensures that only system() is called while proc_exit() might be called in the SIGTERM handler, which is how this code worked from v8.4 to v14. --- src/backend/access/transam/xlogarchive.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index f3fb92c8f9..b998cd651c 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -159,20 +159,27 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); + fflush(NULL); + pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); + /* - * Check signals before restore command and reset afterwards. + * PreRestoreCommand() informs the SIGTERM handler for the startup process + * that it should proc_exit() right away. This is done for the duration of + * the system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, it + * is best to put any additional logic before or after the + * PreRestoreCommand()/PostRestoreCommand() section. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); rc = system(xlogRestoreCmd); - pgstat_report_wait_end(); PostRestoreCommand(); + + pgstat_report_wait_end(); pfree(xlogRestoreCmd); if (rc == 0) -- 2.25.1 --jRHKVT23PllUwdXP Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0002-Don-t-proc_exit-in-startup-s-SIGTERM-handler-if-.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations @ 2025-02-07 13:01 Sutou Kouhei <[email protected]> 2025-02-20 23:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Sutou Kouhei @ 2025-02-07 13:01 UTC (permalink / raw) To: [email protected]; +Cc: [email protected]; [email protected]; pgsql-hackers Hi, In <CAD21AoBkDE4JwjPgcLxSEwqu3nN4VXjkYS9vpRQDwA2GwNQCsg@mail.gmail.com> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Tue, 4 Feb 2025 22:20:51 -0800, Masahiko Sawada <[email protected]> wrote: >> I was just looking at bit at this series of patch labelled with v31, >> to see what is happening here. >> >> In 0001, we have that: >> >> + /* format-specific routines */ >> + const CopyToRoutine *routine; >> [...] >> - CopySendEndOfRow(cstate); >> + cstate->routine->CopyToOneRow(cstate, slot); >> >> Having a callback where the copy state is processed once per row is >> neat in terms of design for the callbacks and what extensions can do, >> and this is much better than what 2889fd23be5 has attempted (later >> reverted in 1aa8324b81fa) because we don't do indirect function calls >> for each attribute. Still, I have a question here: what happens for a >> COPY TO that involves one attribute, a short field size like an int2 >> and many rows (the more rows the more pronounced the effect, of >> course)? Could this level of indirection still be the cause of some >> regressions in a case like that? This is the worst case I can think >> about, on top of my mind, and I am not seeing tests with few >> attributes like this one, where we would try to make this callback as >> hot as possible. This is a performance-sensitive area. > > FYI when Sutou-san last measured the performance[1], it showed a > slight speed up even with fewer columns (5 columns) in both COPY TO > and COPY FROM cases. The callback design has not changed since then. > But it would be a good idea to run the benchmark with a table having a > single small size column. > > [1] https://www.postgresql.org/message-id/20241114.161948.1677325020727842666.kou%40clear-code.com I measured v31 patch set with 1,6,11,16,21,26,31 int2 columns. See the attached PDF for 0001 and 0002 result. 0001 - to: It's faster than master when the number of rows are 1,000,000-5,000,000. It's almost same as master when the number of rows are 6,000,000-10,000,000. There is no significant slow down when the number of columns is 1. 0001 - from: 0001 doesn't change COPY FROM code. So the differences are not real difference. 0002 - to: 0002 doesn't change COPY TO code. So "0001 - to" and "0002 - to" must be the same result. But 0002 is faster than master for all cases. It shows that the CopyToOneRow() approach doesn't have significant slow down. 0002 - from: 0002 changes COPY FROM code. So this may have performance impact. It's almost same as master when data is smaller ((1,000,000-2,000,000 rows) or (3,000,000 rows and 1,6,11,16 columns)). It's faster than master when data is larger. There is no significant slow down by 0002. Thanks, -- kou Attachments: [application/pdf] v31-intel-core-i7-3770-result-1-2.pdf (53.4K, ../../[email protected]/2-v31-intel-core-i7-3770-result-1-2.pdf) download ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations 2025-02-07 13:01 Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> @ 2025-02-20 23:28 ` Masahiko Sawada <[email protected]> 2025-02-21 02:48 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Masahiko Sawada @ 2025-02-20 23:28 UTC (permalink / raw) To: Sutou Kouhei <[email protected]>; +Cc: [email protected]; [email protected]; pgsql-hackers On Fri, Feb 7, 2025 at 5:01 AM Sutou Kouhei <[email protected]> wrote: > > Hi, > > In <CAD21AoBkDE4JwjPgcLxSEwqu3nN4VXjkYS9vpRQDwA2GwNQCsg@mail.gmail.com> > "Re: Make COPY format extendable: Extract COPY TO format implementations" on Tue, 4 Feb 2025 22:20:51 -0800, > Masahiko Sawada <[email protected]> wrote: > > >> I was just looking at bit at this series of patch labelled with v31, > >> to see what is happening here. > >> > >> In 0001, we have that: > >> > >> + /* format-specific routines */ > >> + const CopyToRoutine *routine; > >> [...] > >> - CopySendEndOfRow(cstate); > >> + cstate->routine->CopyToOneRow(cstate, slot); > >> > >> Having a callback where the copy state is processed once per row is > >> neat in terms of design for the callbacks and what extensions can do, > >> and this is much better than what 2889fd23be5 has attempted (later > >> reverted in 1aa8324b81fa) because we don't do indirect function calls > >> for each attribute. Still, I have a question here: what happens for a > >> COPY TO that involves one attribute, a short field size like an int2 > >> and many rows (the more rows the more pronounced the effect, of > >> course)? Could this level of indirection still be the cause of some > >> regressions in a case like that? This is the worst case I can think > >> about, on top of my mind, and I am not seeing tests with few > >> attributes like this one, where we would try to make this callback as > >> hot as possible. This is a performance-sensitive area. > > > > FYI when Sutou-san last measured the performance[1], it showed a > > slight speed up even with fewer columns (5 columns) in both COPY TO > > and COPY FROM cases. The callback design has not changed since then. > > But it would be a good idea to run the benchmark with a table having a > > single small size column. > > > > [1] https://www.postgresql.org/message-id/20241114.161948.1677325020727842666.kou%40clear-code.com > > I measured v31 patch set with 1,6,11,16,21,26,31 int2 > columns. See the attached PDF for 0001 and 0002 result. > > 0001 - to: > > It's faster than master when the number of rows are > 1,000,000-5,000,000. > > It's almost same as master when the number of rows are > 6,000,000-10,000,000. > > There is no significant slow down when the number of columns > is 1. > > 0001 - from: > > 0001 doesn't change COPY FROM code. So the differences are > not real difference. > > 0002 - to: > > 0002 doesn't change COPY TO code. So "0001 - to" and "0002 - > to" must be the same result. But 0002 is faster than master > for all cases. It shows that the CopyToOneRow() approach > doesn't have significant slow down. > > 0002 - from: > > 0002 changes COPY FROM code. So this may have performance > impact. > > It's almost same as master when data is smaller > ((1,000,000-2,000,000 rows) or (3,000,000 rows and 1,6,11,16 > columns)). > > It's faster than master when data is larger. > > There is no significant slow down by 0002. > Thank you for sharing the benchmark results. That looks good to me. Looking at the 0001 patch again, I have a question: we have CopyToTextLikeOneRow() for both CSV and text format: +/* Implementation of the per-row callback for text format */ +static void +CopyToTextOneRow(CopyToState cstate, TupleTableSlot *slot) +{ + CopyToTextLikeOneRow(cstate, slot, false); +} + +/* Implementation of the per-row callback for CSV format */ +static void +CopyToCSVOneRow(CopyToState cstate, TupleTableSlot *slot) +{ + CopyToTextLikeOneRow(cstate, slot, true); +} These two functions pass different is_csv value to that function, which is used as follows: + if (is_csv) + CopyAttributeOutCSV(cstate, string, + cstate->opts.force_quote_flags[attnum - 1]); + else + CopyAttributeOutText(cstate, string); However, we can know whether the format is CSV or text by checking cstate->opts.csv_mode instead of passing is_csv. That way, we can directly call CopyToTextLikeOneRow() but not via CopyToCSVOneRow() or CopyToTextOneRow(). It would not help performance since we already inline CopyToTextLikeOneRow(), but it looks simpler. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations 2025-02-07 13:01 Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-20 23:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> @ 2025-02-21 02:48 ` Sutou Kouhei <[email protected]> 2025-02-25 22:05 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Sutou Kouhei @ 2025-02-21 02:48 UTC (permalink / raw) To: [email protected]; +Cc: [email protected]; [email protected]; pgsql-hackers Hi, In <CAD21AoAni3cKToPfdShTsc0NmaJOtbJuUb=skyz3Udj7HZY7dA@mail.gmail.com> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Thu, 20 Feb 2025 15:28:26 -0800, Masahiko Sawada <[email protected]> wrote: > Looking at the 0001 patch again, I have a question: we have > CopyToTextLikeOneRow() for both CSV and text format: > > +/* Implementation of the per-row callback for text format */ > +static void > +CopyToTextOneRow(CopyToState cstate, TupleTableSlot *slot) > +{ > + CopyToTextLikeOneRow(cstate, slot, false); > +} > + > +/* Implementation of the per-row callback for CSV format */ > +static void > +CopyToCSVOneRow(CopyToState cstate, TupleTableSlot *slot) > +{ > + CopyToTextLikeOneRow(cstate, slot, true); > +} > > These two functions pass different is_csv value to that function, > which is used as follows: > > + if (is_csv) > + CopyAttributeOutCSV(cstate, string, > + > cstate->opts.force_quote_flags[attnum - 1]); > + else > + CopyAttributeOutText(cstate, string); > > However, we can know whether the format is CSV or text by checking > cstate->opts.csv_mode instead of passing is_csv. That way, we can > directly call CopyToTextLikeOneRow() but not via CopyToCSVOneRow() or > CopyToTextOneRow(). It would not help performance since we already > inline CopyToTextLikeOneRow(), but it looks simpler. This means the following, right? 1. We remove CopyToTextOneRow() and CopyToCSVOneRow() 2. We remove "bool is_csv" parameter from CopyToTextLikeOneRow() and use cstate->opts.csv_mode in CopyToTextLikeOneRow() instead of is_csv 3. We use CopyToTextLikeOneRow() for CopyToRoutineText::CopyToOneRow and CopyToRoutineCSV::CopyToOneRow If we use this approach, we can't remove the following branch in compile time: + if (is_csv) + CopyAttributeOutCSV(cstate, string, + cstate->opts.force_quote_flags[attnum - 1]); + else + CopyAttributeOutText(cstate, string); We can remove the branch in compile time with the current approach (constant argument + inline). It may have a negative performance impact because the "if" is used many times with large data. (That's why we choose the constant argument + inline approach in this thread.) Thanks, -- kou ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations 2025-02-07 13:01 Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-20 23:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 2025-02-21 02:48 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> @ 2025-02-25 22:05 ` Masahiko Sawada <[email protected]> 2025-02-25 23:51 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Masahiko Sawada @ 2025-02-25 22:05 UTC (permalink / raw) To: Sutou Kouhei <[email protected]>; +Cc: [email protected]; [email protected]; pgsql-hackers On Thu, Feb 20, 2025 at 6:48 PM Sutou Kouhei <[email protected]> wrote: > > Hi, > > In <CAD21AoAni3cKToPfdShTsc0NmaJOtbJuUb=skyz3Udj7HZY7dA@mail.gmail.com> > "Re: Make COPY format extendable: Extract COPY TO format implementations" on Thu, 20 Feb 2025 15:28:26 -0800, > Masahiko Sawada <[email protected]> wrote: > > > Looking at the 0001 patch again, I have a question: we have > > CopyToTextLikeOneRow() for both CSV and text format: > > > > +/* Implementation of the per-row callback for text format */ > > +static void > > +CopyToTextOneRow(CopyToState cstate, TupleTableSlot *slot) > > +{ > > + CopyToTextLikeOneRow(cstate, slot, false); > > +} > > + > > +/* Implementation of the per-row callback for CSV format */ > > +static void > > +CopyToCSVOneRow(CopyToState cstate, TupleTableSlot *slot) > > +{ > > + CopyToTextLikeOneRow(cstate, slot, true); > > +} > > > > These two functions pass different is_csv value to that function, > > which is used as follows: > > > > + if (is_csv) > > + CopyAttributeOutCSV(cstate, string, > > + > > cstate->opts.force_quote_flags[attnum - 1]); > > + else > > + CopyAttributeOutText(cstate, string); > > > > However, we can know whether the format is CSV or text by checking > > cstate->opts.csv_mode instead of passing is_csv. That way, we can > > directly call CopyToTextLikeOneRow() but not via CopyToCSVOneRow() or > > CopyToTextOneRow(). It would not help performance since we already > > inline CopyToTextLikeOneRow(), but it looks simpler. > > This means the following, right? > > 1. We remove CopyToTextOneRow() and CopyToCSVOneRow() > 2. We remove "bool is_csv" parameter from CopyToTextLikeOneRow() > and use cstate->opts.csv_mode in CopyToTextLikeOneRow() > instead of is_csv > 3. We use CopyToTextLikeOneRow() for > CopyToRoutineText::CopyToOneRow and > CopyToRoutineCSV::CopyToOneRow > > If we use this approach, we can't remove the following > branch in compile time: > > + if (is_csv) > + CopyAttributeOutCSV(cstate, string, > + cstate->opts.force_quote_flags[attnum - 1]); > + else > + CopyAttributeOutText(cstate, string); > > We can remove the branch in compile time with the current > approach (constant argument + inline). > > It may have a negative performance impact because the "if" > is used many times with large data. (That's why we choose > the constant argument + inline approach in this thread.) > Thank you for the explanation, I missed that fact. I'm fine with having is_csv. The first two patches are refactoring patches (+ small performance improvements). I've reviewed these patches again and attached the updated patches. I reorganized the function order and updated comments etc. I find that these patches are reasonably ready to push. Could you review these versions? I'm going to push them, barring objections and further comments. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com Attachments: [application/octet-stream] v33-0002-Refactor-COPY-FROM-to-use-format-callback-functi.patch (32.5K, ../../CAD21AoBjzkL2Lv7j4teaHBZvNmKctQtH6X71kN_sj6Fm-+VvJQ@mail.gmail.com/2-v33-0002-Refactor-COPY-FROM-to-use-format-callback-functi.patch) download | inline diff: From 1d2873a9e33fe2784aa6131e8b3973be9afe81b3 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada <[email protected]> Date: Tue, 25 Feb 2025 13:58:58 -0800 Subject: [PATCH v33 2/2] Refactor COPY FROM to use format callback functions. This commit introduces a new CopyFromRoutine struct, which is a set of callback routines to read tuples in a specific format. It also makes COPY FROM with the existing formats (text, CSV, and binary) utilize these format callbacks. This change is a preliminary step towards making the COPY TO command extensible in terms of output formats. Similar to XXXX, this refactoring contributes to a performance improvement by reducing the number of "if" branches that need to be checked on a per-row basis when sending field representations in text or CSV mode. The performance benchmark results showed ~5% performance gain in text or CSV mode. Author: Sutou Kouhei <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Andres Freund <[email protected]> Reviewed-by: Tomas Vondra <[email protected]> Reviewed-by: Junwang Zhao <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/copyfrom.c | 192 +++++++--- src/backend/commands/copyfromparse.c | 443 +++++++++++++---------- src/include/commands/copy.h | 2 - src/include/commands/copyapi.h | 48 ++- src/include/commands/copyfrom_internal.h | 11 + src/tools/pgindent/typedefs.list | 1 + 6 files changed, 453 insertions(+), 244 deletions(-) diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 8875d79d59a..4c2479974c4 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -28,7 +28,7 @@ #include "access/tableam.h" #include "access/xact.h" #include "catalog/namespace.h" -#include "commands/copy.h" +#include "commands/copyapi.h" #include "commands/copyfrom_internal.h" #include "commands/progress.h" #include "commands/trigger.h" @@ -106,6 +106,145 @@ typedef struct CopyMultiInsertInfo /* non-export function prototypes */ static void ClosePipeFromProgram(CopyFromState cstate); +/* + * Built-in format-specific routines. One-row callbacks are defined in + * copyfromparse.c + */ +static void CopyFromTextLikeInFunc(CopyFromState cstate, Oid atttypid, FmgrInfo *finfo, + Oid *typioparam); +static void CopyFromTextLikeStart(CopyFromState cstate, TupleDesc tupDesc); +static void CopyFromTextLikeEnd(CopyFromState cstate); +static void CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid, + FmgrInfo *finfo, Oid *typioparam); +static void CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc); +static void CopyFromBinaryEnd(CopyFromState cstate); + + +/* + * COPY FROM routines for built-in formats. + * + * CSV and text formats share the same TextLike routines except for the + * one-row callback. + */ + +/* text format */ +static const CopyFromRoutine CopyFromRoutineText = { + .CopyFromInFunc = CopyFromTextLikeInFunc, + .CopyFromStart = CopyFromTextLikeStart, + .CopyFromOneRow = CopyFromTextOneRow, + .CopyFromEnd = CopyFromTextLikeEnd, +}; + +/* CSV format */ +static const CopyFromRoutine CopyFromRoutineCSV = { + .CopyFromInFunc = CopyFromTextLikeInFunc, + .CopyFromStart = CopyFromTextLikeStart, + .CopyFromOneRow = CopyFromCSVOneRow, + .CopyFromEnd = CopyFromTextLikeEnd, +}; + +/* binary format */ +static const CopyFromRoutine CopyFromRoutineBinary = { + .CopyFromInFunc = CopyFromBinaryInFunc, + .CopyFromStart = CopyFromBinaryStart, + .CopyFromOneRow = CopyFromBinaryOneRow, + .CopyFromEnd = CopyFromBinaryEnd, +}; + +/* Return a COPY FROM routine for the given options */ +static const CopyFromRoutine * +CopyFromGetRoutine(CopyFormatOptions opts) +{ + if (opts.csv_mode) + return &CopyFromRoutineCSV; + else if (opts.binary) + return &CopyFromRoutineBinary; + + /* default is text */ + return &CopyFromRoutineText; +} + +/* Implementation of the start callback for text and CSV formats */ +static void +CopyFromTextLikeStart(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; used by CSV and text + * format. + */ + attr_count = list_length(cstate->attnumlist); + cstate->max_fields = attr_count; + cstate->raw_fields = (char **) palloc(attr_count * sizeof(char *)); +} + +/* + * Implementation of the infunc callback for text and CSV formats. Assign + * the input function data to the given *finfo. + */ +static void +CopyFromTextLikeInFunc(CopyFromState cstate, Oid atttypid, FmgrInfo *finfo, + Oid *typioparam) +{ + Oid func_oid; + + getTypeInputInfo(atttypid, &func_oid, typioparam); + fmgr_info(func_oid, finfo); +} + +/* Implementation of the end callback for text and CSV formats */ +static void +CopyFromTextLikeEnd(CopyFromState cstate) +{ + /* nothing to do */ +} + +/* Implementation of the start callback for binary format */ +static void +CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc) +{ + /* Read and verify binary header */ + ReceiveCopyBinaryHeader(cstate); +} + +/* + * Implementation of the infunc callback for binary format. Assign + * the binary input function to the given *finfo. + */ +static void +CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid, + FmgrInfo *finfo, Oid *typioparam) +{ + Oid func_oid; + + getTypeBinaryInputInfo(atttypid, &func_oid, typioparam); + fmgr_info(func_oid, finfo); +} + +/* Implementation of the end callback for binary format */ +static void +CopyFromBinaryEnd(CopyFromState cstate) +{ + /* nothing to do */ +} + /* * error context callback for COPY FROM * @@ -1403,7 +1542,6 @@ BeginCopyFrom(ParseState *pstate, num_defaults; FmgrInfo *in_functions; Oid *typioparams; - Oid in_func_oid; int *defmap; ExprState **defexprs; MemoryContext oldcontext; @@ -1435,6 +1573,9 @@ BeginCopyFrom(ParseState *pstate, /* Extract options from the statement node tree */ ProcessCopyOptions(pstate, &cstate->opts, true /* is_from */ , options); + /* Set the format routine */ + cstate->routine = CopyFromGetRoutine(cstate->opts); + /* Process the target relation */ cstate->rel = rel; @@ -1590,25 +1731,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. */ @@ -1641,13 +1763,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; @@ -1782,20 +1900,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 *)); - } + cstate->routine->CopyFromStart(cstate, tupDesc); MemoryContextSwitchTo(oldcontext); @@ -1808,6 +1913,9 @@ BeginCopyFrom(ParseState *pstate, void EndCopyFrom(CopyFromState cstate) { + /* Invoke the end callback */ + 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 caccdc8563c..d5d5fda0ae5 100644 --- a/src/backend/commands/copyfromparse.c +++ b/src/backend/commands/copyfromparse.c @@ -62,7 +62,7 @@ #include <unistd.h> #include <sys/stat.h> -#include "commands/copy.h" +#include "commands/copyapi.h" #include "commands/copyfrom_internal.h" #include "commands/progress.h" #include "executor/executor.h" @@ -140,8 +140,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 bool CopyReadLine(CopyFromState cstate, bool is_csv); +static bool CopyReadLineText(CopyFromState cstate, bool is_csv); static int CopyReadAttributesText(CopyFromState cstate); static int CopyReadAttributesCSV(CopyFromState cstate); static Datum CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo, @@ -740,9 +740,11 @@ CopyReadBinaryData(CopyFromState cstate, char *dest, int nbytes) * in the relation. * * NOTE: force_not_null option are not applied to the returned fields. + * + * We use pg_attribute_always_inline to reduce function call overheads. */ -bool -NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) +static pg_attribute_always_inline bool +NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields, bool is_csv) { int fldct; bool done; @@ -759,13 +761,13 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) tupDesc = RelationGetDescr(cstate->rel); cstate->cur_lineno++; - done = CopyReadLine(cstate); + done = CopyReadLine(cstate, is_csv); if (cstate->opts.header_line == COPY_HEADER_MATCH) { int fldnum; - if (cstate->opts.csv_mode) + if (is_csv) fldct = CopyReadAttributesCSV(cstate); else fldct = CopyReadAttributesText(cstate); @@ -809,7 +811,7 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) cstate->cur_lineno++; /* Actually read the line into memory here */ - done = CopyReadLine(cstate); + done = CopyReadLine(cstate, is_csv); /* * EOF at start of line means we're done. If we see EOF after some @@ -819,8 +821,10 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) if (done && cstate->line_buf.len == 0) return false; - /* Parse the line into de-escaped field values */ - if (cstate->opts.csv_mode) + /* + * Parse the line into de-escaped field values. + */ + if (is_csv) fldct = CopyReadAttributesCSV(cstate); else fldct = CopyReadAttributesText(cstate); @@ -847,233 +851,274 @@ 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) + /* Get one row from source */ + if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls)) + return false; + + /* + * Now compute and insert any defaults available for the columns not + * provided by the input data. Anything not processed here or above will + * remain NULL. + */ + for (i = 0; i < num_defaults; i++) { - char **field_strings; - ListCell *cur; - int fldct; - int fieldno; - char *string; + /* + * 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); - /* read raw fields in the next line */ - if (!NextCopyFromRawFields(cstate, &field_strings, &fldct)) - return false; + values[defmap[i]] = ExecEvalExpr(defexprs[defmap[i]], econtext, + &nulls[defmap[i]]); + } + + return true; +} + +/* Implementation of the per-row callback for text format */ +bool +CopyFromTextOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values, + bool *nulls) +{ + return CopyFromTextLikeOneRow(cstate, econtext, values, nulls, false); +} + +/* Implementation of the per-row callback for CSV format */ +bool +CopyFromCSVOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values, + bool *nulls) +{ + return CopyFromTextLikeOneRow(cstate, econtext, values, nulls, true); +} + +/* + * Workhorse for CopyFromTextOneRow() and CopyFromCSVOneRow(). + * + * We use pg_attribute_always_inline to reduce function call overheads. + */ +static pg_attribute_always_inline bool +CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext, + Datum *values, bool *nulls, bool is_csv) +{ + 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, is_csv)) + 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); - /* check for overflowing fields */ - if (attr_count > 0 && fldct > attr_count) + if (fieldno >= fldct) ereport(ERROR, (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), - errmsg("extra data after last expected column"))); - - fieldno = 0; + errmsg("missing data for column \"%s\"", + NameStr(att->attname)))); + string = field_strings[fieldno++]; - /* Loop to read the user attributes on the line. */ - foreach(cur, cstate->attnumlist) + if (cstate->convert_select_flags && + !cstate->convert_select_flags[m]) { - 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++]; + /* ignore input field, leaving column as NULL */ + continue; + } - if (cstate->convert_select_flags && - !cstate->convert_select_flags[m]) + if (is_csv) + { + if (string == NULL && + cstate->opts.force_notnull_flags[m]) { - /* ignore input field, leaving column as NULL */ - continue; + /* + * FORCE_NOT_NULL option is set and column is NULL - convert + * it to the NULL string. + */ + string = cstate->opts.null_print; } - - if (cstate->opts.csv_mode) + else if (string != NULL && cstate->opts.force_null_flags[m] + && strcmp(string, cstate->opts.null_print) == 0) { - 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; - } + /* + * 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; + cstate->cur_attname = NameStr(att->attname); + cstate->cur_attval = string; - if (string != NULL) - nulls[m] = false; + 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); + if (cstate->defaults[m]) + { + /* We must have switched into the per-tuple memory context */ + Assert(econtext != NULL); + Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory); - values[m] = ExecEvalExpr(defexprs[m], econtext, &nulls[m]); - } + 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])) - { - Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP); + /* + * 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])) + { + Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP); - cstate->num_errors++; + cstate->num_errors++; - if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE) - { - /* - * Since we emit line number and column info in the below - * notice message, we suppress error context information - * other than the relation name. - */ - Assert(!cstate->relname_only); - cstate->relname_only = true; + if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE) + { + /* + * Since we emit line number and column info in the below + * notice message, we suppress error context information other + * than the relation name. + */ + Assert(!cstate->relname_only); + cstate->relname_only = true; - if (cstate->cur_attval) - { - char *attval; - - attval = CopyLimitPrintoutLength(cstate->cur_attval); - ereport(NOTICE, - errmsg("skipping row due to data type incompatibility at line %llu for column \"%s\": \"%s\"", - (unsigned long long) cstate->cur_lineno, - cstate->cur_attname, - attval)); - pfree(attval); - } - else - ereport(NOTICE, - errmsg("skipping row due to data type incompatibility at line %llu for column \"%s\": null input", - (unsigned long long) cstate->cur_lineno, - cstate->cur_attname)); - - /* reset relname_only */ - cstate->relname_only = false; + if (cstate->cur_attval) + { + char *attval; + + attval = CopyLimitPrintoutLength(cstate->cur_attval); + ereport(NOTICE, + errmsg("skipping row due to data type incompatibility at line %llu for column \"%s\": \"%s\"", + (unsigned long long) cstate->cur_lineno, + cstate->cur_attname, + attval)); + pfree(attval); } + else + ereport(NOTICE, + errmsg("skipping row due to data type incompatibility at line %llu for column \"%s\": null input", + (unsigned long long) cstate->cur_lineno, + cstate->cur_attname)); - return true; + /* reset relname_only */ + cstate->relname_only = false; } - cstate->cur_attname = NULL; - cstate->cur_attval = NULL; + return true; } - Assert(fieldno == attr_count); + cstate->cur_attname = NULL; + cstate->cur_attval = NULL; } - else - { - /* binary */ - int16 fld_count; - ListCell *cur; - cstate->cur_lineno++; + Assert(fieldno == attr_count); - if (!CopyGetInt16(cstate, &fld_count)) - { - /* EOF detected (end of file, or protocol-level EOF) */ - return false; - } + return true; +} - 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; +/* Implementation of the per-row callback for 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; - if (CopyReadBinaryData(cstate, &dummy, 1) > 0) - ereport(ERROR, - (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), - errmsg("received copy data after EOF marker"))); - return false; - } + tupDesc = RelationGetDescr(cstate->rel); + attr_count = list_length(cstate->attnumlist); - 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))); + cstate->cur_lineno++; - 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 (!CopyGetInt16(cstate, &fld_count)) + { + /* EOF detected (end of file, or protocol-level EOF) */ + return false; } - /* - * Now compute and insert any defaults available for the columns not - * provided by the input data. Anything not processed here or above will - * remain NULL. - */ - for (i = 0; i < num_defaults; i++) + if (fld_count == -1) { /* - * The caller must supply econtext and have switched into the - * per-tuple memory context in it. + * 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. */ - Assert(econtext != NULL); - Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory); + char dummy; - values[defmap[i]] = ExecEvalExpr(defexprs[defmap[i]], econtext, - &nulls[defmap[i]]); + 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; @@ -1087,7 +1132,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, * in the final value of line_buf. */ static bool -CopyReadLine(CopyFromState cstate) +CopyReadLine(CopyFromState cstate, bool is_csv) { bool result; @@ -1095,7 +1140,7 @@ CopyReadLine(CopyFromState cstate) cstate->line_buf_valid = false; /* Parse data and transfer into line_buf */ - result = CopyReadLineText(cstate); + result = CopyReadLineText(cstate, is_csv); if (result) { @@ -1163,7 +1208,7 @@ CopyReadLine(CopyFromState cstate) * CopyReadLineText - inner loop of CopyReadLine for text mode */ static bool -CopyReadLineText(CopyFromState cstate) +CopyReadLineText(CopyFromState cstate, bool is_csv) { char *copy_input_buf; int input_buf_ptr; @@ -1178,7 +1223,7 @@ CopyReadLineText(CopyFromState cstate) char quotec = '\0'; char escapec = '\0'; - if (cstate->opts.csv_mode) + if (is_csv) { quotec = cstate->opts.quote[0]; escapec = cstate->opts.escape[0]; @@ -1255,7 +1300,7 @@ CopyReadLineText(CopyFromState cstate) prev_raw_ptr = input_buf_ptr; c = copy_input_buf[input_buf_ptr++]; - if (cstate->opts.csv_mode) + if (is_csv) { /* * If character is '\r', we may need to look ahead below. Force @@ -1294,7 +1339,7 @@ CopyReadLineText(CopyFromState cstate) } /* Process \r */ - if (c == '\r' && (!cstate->opts.csv_mode || !in_quote)) + if (c == '\r' && (!is_csv || !in_quote)) { /* Check for \r\n on first line, _and_ handle \r\n. */ if (cstate->eol_type == EOL_UNKNOWN || @@ -1322,10 +1367,10 @@ CopyReadLineText(CopyFromState cstate) if (cstate->eol_type == EOL_CRNL) ereport(ERROR, (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), - !cstate->opts.csv_mode ? + !is_csv ? errmsg("literal carriage return found in data") : errmsg("unquoted carriage return found in data"), - !cstate->opts.csv_mode ? + !is_csv ? errhint("Use \"\\r\" to represent carriage return.") : errhint("Use quoted CSV field to represent carriage return."))); @@ -1339,10 +1384,10 @@ CopyReadLineText(CopyFromState cstate) else if (cstate->eol_type == EOL_NL) ereport(ERROR, (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), - !cstate->opts.csv_mode ? + !is_csv ? errmsg("literal carriage return found in data") : errmsg("unquoted carriage return found in data"), - !cstate->opts.csv_mode ? + !is_csv ? 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 */ @@ -1350,15 +1395,15 @@ CopyReadLineText(CopyFromState cstate) } /* Process \n */ - if (c == '\n' && (!cstate->opts.csv_mode || !in_quote)) + if (c == '\n' && (!is_csv || !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 ? + !is_csv ? errmsg("literal newline found in data") : errmsg("unquoted newline found in data"), - !cstate->opts.csv_mode ? + !is_csv ? errhint("Use \"\\n\" to represent newline.") : errhint("Use quoted CSV field to represent newline."))); cstate->eol_type = EOL_NL; /* in case not set yet */ @@ -1370,7 +1415,7 @@ CopyReadLineText(CopyFromState cstate) * Process backslash, except in CSV mode where backslash is a normal * character. */ - if (c == '\\' && !cstate->opts.csv_mode) + if (c == '\\' && !is_csv) { char c2; diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h index 06dfdfef721..7bc044e2816 100644 --- a/src/include/commands/copy.h +++ b/src/include/commands/copy.h @@ -107,8 +107,6 @@ extern CopyFromState BeginCopyFrom(ParseState *pstate, Relation rel, Node *where extern void EndCopyFrom(CopyFromState cstate); extern bool NextCopyFrom(CopyFromState cstate, ExprContext *econtext, Datum *values, bool *nulls); -extern bool NextCopyFromRawFields(CopyFromState cstate, - char ***fields, int *nfields); extern void CopyFromErrorCallback(void *arg); extern char *CopyLimitPrintoutLength(const char *str); diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h index eb09271d94b..fc114079d5a 100644 --- a/src/include/commands/copyapi.h +++ b/src/include/commands/copyapi.h @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * copyapi.h - * API for COPY TO handlers + * API for COPY TO/FROM handlers * * * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group @@ -52,4 +52,50 @@ typedef struct CopyToRoutine void (*CopyToEnd) (CopyToState cstate); } CopyToRoutine; +/* + * 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 +{ + /* + * Set input function information. This callback is called once at the + * beginning of COPY FROM. + * + * '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); + + /* + * Start a COPY FROM. This callback is called once at the beginning of + * COPY FROM. + * + * '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); + + /* + * Read one row from the source and fill *values and *nulls. + * + * '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 read. + */ + bool (*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext, + Datum *values, bool *nulls); + + /* End a COPY FROM. This callback is called once at the end of COPY FROM */ + void (*CopyFromEnd) (CopyFromState cstate); +} CopyFromRoutine; + #endif /* COPYAPI_H */ diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h index 1d8ac8f62e6..c8b22af22d8 100644 --- a/src/include/commands/copyfrom_internal.h +++ b/src/include/commands/copyfrom_internal.h @@ -58,6 +58,9 @@ typedef enum CopyInsertMethod */ typedef struct CopyFromStateData { + /* format routine */ + const struct CopyFromRoutine *routine; + /* low-level state data */ CopySource copy_src; /* type of copy source */ FILE *copy_file; /* used if copy_src == COPY_FILE */ @@ -183,4 +186,12 @@ typedef struct CopyFromStateData extern void ReceiveCopyBegin(CopyFromState cstate); extern void ReceiveCopyBinaryHeader(CopyFromState cstate); +/* One-row callbacks for built-in formats defined in copyfromparse.c */ +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); + #endif /* COPYFROM_INTERNAL_H */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index b0c97b0fb85..eca93ab9553 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -501,6 +501,7 @@ ConvertRowtypeExpr CookedConstraint CopyDest CopyFormatOptions +CopyFromRoutine CopyFromState CopyFromStateData CopyHeaderChoice -- 2.43.5 [application/octet-stream] v33-0001-Refactor-COPY-TO-to-use-format-callback-function.patch (18.0K, ../../CAD21AoBjzkL2Lv7j4teaHBZvNmKctQtH6X71kN_sj6Fm-+VvJQ@mail.gmail.com/3-v33-0001-Refactor-COPY-TO-to-use-format-callback-function.patch) download | inline diff: From 6c2d95044ef89295bde30ccb0a0dbedeb6cb94ca Mon Sep 17 00:00:00 2001 From: Masahiko Sawada <[email protected]> Date: Tue, 25 Feb 2025 13:58:41 -0800 Subject: [PATCH v33 1/2] Refactor COPY TO to use format callback functions. This commit introduces a new CopyToRoutine struct, which is a set of callback routines to copy tuples in a specific format. It also makes the existing formats (text, CSV, and binary) utilize these format callbacks. This change is a preliminary step towards making the COPY TO command extensible in terms of output formats. Additionally, this refactoring contributes to a performance improvement by reducing the number of "if" branches that need to be checked on a per-row basis when sending field representations in text or CSV mode. The performance benchmark results showed ~5% performance gain intext or CSV mode. Author: Sutou Kouhei <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Andres Freund <[email protected]> Reviewed-by: Tomas Vondra <[email protected]> Reviewed-by: Junwang Zhao <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/copyto.c | 440 +++++++++++++++++++++---------- src/include/commands/copyapi.h | 55 ++++ src/tools/pgindent/typedefs.list | 1 + 3 files changed, 356 insertions(+), 140 deletions(-) create mode 100644 src/include/commands/copyapi.h diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 091fbc12cc5..a74a7e763dc 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -20,6 +20,7 @@ #include "access/tableam.h" #include "commands/copy.h" +#include "commands/copyapi.h" #include "commands/progress.h" #include "executor/execdesc.h" #include "executor/executor.h" @@ -64,6 +65,9 @@ typedef enum CopyDest */ typedef struct CopyToStateData { + /* format-specific routines */ + const CopyToRoutine *routine; + /* low-level state data */ CopyDest copy_dest; /* type of copy source/destination */ FILE *copy_file; /* used if copy_dest == COPY_FILE */ @@ -114,6 +118,19 @@ static void CopyAttributeOutText(CopyToState cstate, const char *string); static void CopyAttributeOutCSV(CopyToState cstate, const char *string, bool use_quote); +/* built-in format-specific routines */ +static void CopyToTextLikeStart(CopyToState cstate, TupleDesc tupDesc); +static void CopyToTextLikeOutFunc(CopyToState cstate, Oid atttypid, FmgrInfo *finfo); +static void CopyToTextOneRow(CopyToState cstate, TupleTableSlot *slot); +static void CopyToCSVOneRow(CopyToState cstate, TupleTableSlot *slot); +static void CopyToTextLikeOneRow(CopyToState cstate, TupleTableSlot *slot, + bool is_csv); +static void CopyToTextLikeEnd(CopyToState cstate); +static void CopyToBinaryStart(CopyToState cstate, TupleDesc tupDesc); +static void CopyToBinaryOutFunc(CopyToState cstate, Oid atttypid, FmgrInfo *finfo); +static void CopyToBinaryOneRow(CopyToState cstate, TupleTableSlot *slot); +static void CopyToBinaryEnd(CopyToState cstate); + /* Low-level communications functions */ static void SendCopyBegin(CopyToState cstate); static void SendCopyEnd(CopyToState cstate); @@ -121,9 +138,254 @@ static void CopySendData(CopyToState cstate, const void *databuf, int datasize); static void CopySendString(CopyToState cstate, const char *str); static void CopySendChar(CopyToState cstate, char c); static void CopySendEndOfRow(CopyToState cstate); +static void CopySendTextLikeEndOfRow(CopyToState cstate); static void CopySendInt32(CopyToState cstate, int32 val); static void CopySendInt16(CopyToState cstate, int16 val); +/* + * COPY TO routines for built-in formats. + * + * CSV and text formats share the same TextLike routines except for the + * one-row callback. + */ + +/* text format */ +static const CopyToRoutine CopyToRoutineText = { + .CopyToStart = CopyToTextLikeStart, + .CopyToOutFunc = CopyToTextLikeOutFunc, + .CopyToOneRow = CopyToTextOneRow, + .CopyToEnd = CopyToTextLikeEnd, +}; + +/* CSV format */ +static const CopyToRoutine CopyToRoutineCSV = { + .CopyToStart = CopyToTextLikeStart, + .CopyToOutFunc = CopyToTextLikeOutFunc, + .CopyToOneRow = CopyToCSVOneRow, + .CopyToEnd = CopyToTextLikeEnd, +}; + +/* binary format */ +static const CopyToRoutine CopyToRoutineBinary = { + .CopyToStart = CopyToBinaryStart, + .CopyToOutFunc = CopyToBinaryOutFunc, + .CopyToOneRow = CopyToBinaryOneRow, + .CopyToEnd = CopyToBinaryEnd, +}; + +/* Return a COPY TO routine for the given options */ +static const CopyToRoutine * +CopyToGetRoutine(CopyFormatOptions opts) +{ + if (opts.csv_mode) + return &CopyToRoutineCSV; + else if (opts.binary) + return &CopyToRoutineBinary; + + /* default is text */ + return &CopyToRoutineText; +} + +/* Implementation of the start callback for text and CSV formats */ +static void +CopyToTextLikeStart(CopyToState cstate, TupleDesc tupDesc) +{ + /* + * 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) + { + ListCell *cur; + 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); + } + + CopySendTextLikeEndOfRow(cstate); + } +} + +/* + * Implementation of the outfunc callback for text and CSV formats. Assign + * the output function data to the given *finfo. + */ +static void +CopyToTextLikeOutFunc(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); +} + +/* Implementation of the per-row callback for text format */ +static void +CopyToTextOneRow(CopyToState cstate, TupleTableSlot *slot) +{ + CopyToTextLikeOneRow(cstate, slot, false); +} + +/* Implementation of the per-row callback for CSV format */ +static void +CopyToCSVOneRow(CopyToState cstate, TupleTableSlot *slot) +{ + CopyToTextLikeOneRow(cstate, slot, true); +} + +/* + * Workhorse for CopyToTextOneRow() and CopyToCSVOneRow(). + * + * We use pg_attribute_always_inline to reduce function call overheads. + */ +static pg_attribute_always_inline void +CopyToTextLikeOneRow(CopyToState cstate, + TupleTableSlot *slot, + bool is_csv) +{ + bool need_delim = false; + FmgrInfo *out_functions = cstate->out_functions; + + foreach_int(attnum, cstate->attnumlist) + { + 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); + + /* + * is_csv will be optimized away by compiler, as argument is + * constant at caller. + */ + if (is_csv) + CopyAttributeOutCSV(cstate, string, + cstate->opts.force_quote_flags[attnum - 1]); + else + CopyAttributeOutText(cstate, string); + } + } + + CopySendTextLikeEndOfRow(cstate); +} + +/* Implementation of the end callback for text and CSV formats */ +static void +CopyToTextLikeEnd(CopyToState cstate) +{ + /* Nothing to do here */ +} + +/* + * Implementation of the start callback for binary format. Send a header + * for a binary copy. + */ +static void +CopyToBinaryStart(CopyToState cstate, TupleDesc tupDesc) +{ + int32 tmp; + + /* Signature */ + CopySendData(cstate, BinarySignature, 11); + /* Flags field */ + tmp = 0; + CopySendInt32(cstate, tmp); + /* No header extension */ + tmp = 0; + CopySendInt32(cstate, tmp); +} + +/* + * Implementation of the outfunc callback for binary format. Assign + * the binary output function to the given *finfo. + */ +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); +} + +/* Implementation of the per-row callback for binary format */ +static void +CopyToBinaryOneRow(CopyToState cstate, TupleTableSlot *slot) +{ + FmgrInfo *out_functions = cstate->out_functions; + + /* Binary per-tuple header */ + CopySendInt16(cstate, list_length(cstate->attnumlist)); + + foreach_int(attnum, cstate->attnumlist) + { + 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); +} + +/* Implementation of the end callback 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); +} /* * Send copy start/stop messages for frontend copies. These have changed @@ -191,16 +453,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)) @@ -235,10 +487,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; @@ -254,6 +502,35 @@ CopySendEndOfRow(CopyToState cstate) resetStringInfo(fe_msgbuf); } +/* + * Wrapper function of CopySendEndOfRow for text and CSV formats. Sends the + * the line termination and do common appropriate things for the end of row. + */ +static inline void +CopySendTextLikeEndOfRow(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); +} + /* * These functions do apply some data conversion */ @@ -426,6 +703,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 +1052,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 +1068,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 +1107,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); @@ -904,74 +1120,18 @@ DoCopyTo(CopyToState cstate) /* * Emit one row during DoCopyTo(). */ -static void +static inline void CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) { - FmgrInfo *out_functions = cstate->out_functions; MemoryContext oldcontext; 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); - if (!cstate->opts.binary) - { - bool need_delim = false; - - foreach_int(attnum, cstate->attnumlist) - { - Datum value = slot->tts_values[attnum - 1]; - bool isnull = slot->tts_isnull[attnum - 1]; - char *string; - - if (need_delim) - CopySendChar(cstate, cstate->opts.delim[0]); - need_delim = true; - - if (isnull) - CopySendString(cstate, cstate->opts.null_print_client); - else - { - 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 - { - foreach_int(attnum, cstate->attnumlist) - { - Datum value = slot->tts_values[attnum - 1]; - bool isnull = slot->tts_isnull[attnum - 1]; - bytea *outputbytes; - - if (isnull) - CopySendInt32(cstate, -1); - else - { - 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/include/commands/copyapi.h b/src/include/commands/copyapi.h new file mode 100644 index 00000000000..eb09271d94b --- /dev/null +++ b/src/include/commands/copyapi.h @@ -0,0 +1,55 @@ +/*------------------------------------------------------------------------- + * + * copyapi.h + * API for COPY TO handlers + * + * + * Portions Copyright (c) 1996-2025, 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 "commands/copy.h" + +/* + * 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 +{ + /* + * Set output function information. This callback is called once at the + * beginning of COPY TO. + * + * 'finfo' can be optionally filled to provide the catalog information of + * the output function. + * + * 'atttypid' is the OID of data type used by the relation's attribute. + */ + void (*CopyToOutFunc) (CopyToState cstate, Oid atttypid, + FmgrInfo *finfo); + + /* + * Start a COPY TO. This callback is called once at the beginning of COPY + * FROM. + * + * 'tupDesc' is the tuple descriptor of the relation from where the data + * is read. + */ + void (*CopyToStart) (CopyToState cstate, TupleDesc tupDesc); + + /* + * Write one row stored in 'slot' to the destination. + */ + void (*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot); + + /* End a COPY TO. This callback is called once at the end of COPY FROM */ + void (*CopyToEnd) (CopyToState cstate); +} CopyToRoutine; + +#endif /* COPYAPI_H */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 1649f5e1011..b0c97b0fb85 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -512,6 +512,7 @@ CopyMultiInsertInfo CopyOnErrorChoice CopySource CopyStmt +CopyToRoutine CopyToState CopyToStateData Cost -- 2.43.5 ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations 2025-02-07 13:01 Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-20 23:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 2025-02-21 02:48 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-25 22:05 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> @ 2025-02-25 23:51 ` Sutou Kouhei <[email protected]> 2025-02-26 01:14 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Sutou Kouhei @ 2025-02-25 23:51 UTC (permalink / raw) To: [email protected]; +Cc: [email protected]; [email protected]; pgsql-hackers Hi, In <CAD21AoBjzkL2Lv7j4teaHBZvNmKctQtH6X71kN_sj6Fm-+VvJQ@mail.gmail.com> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Tue, 25 Feb 2025 14:05:28 -0800, Masahiko Sawada <[email protected]> wrote: > The first two patches are refactoring patches (+ small performance > improvements). I've reviewed these patches again and attached the > updated patches. I reorganized the function order and updated comments > etc. I find that these patches are reasonably ready to push. Could you > review these versions? I'm going to push them, barring objections and > further comments. Sure. Here are some minor comments: 0001: Commit message: > or CSV mode. The performance benchmark results showed ~5% performance > gain intext or CSV mode. intext -> in text > --- a/src/backend/commands/copyto.c > +++ b/src/backend/commands/copyto.c > @@ -20,6 +20,7 @@ > #include "commands/copy.h" > +#include "commands/copyapi.h" We can remove '#include "commands/copy.h"' because it's included in copyapi.h. (0002 does it.) > @@ -254,6 +502,35 @@ CopySendEndOfRow(CopyToState cstate) > +/* > + * Wrapper function of CopySendEndOfRow for text and CSV formats. Sends the > + * the line termination and do common appropriate things for the end of row. > + */ Sends the the line -> Sends the line > --- /dev/null > +++ b/src/include/commands/copyapi.h > + /* End a COPY TO. This callback is called once at the end of COPY FROM */ The last "." is missing: ... COPY FROM. 0002: Commit message: > This change is a preliminary step towards making the COPY TO command > extensible in terms of output formats. COPY TO -> COPY FROM > --- a/src/backend/commands/copyfromparse.c > +++ b/src/backend/commands/copyfromparse.c > @@ -1087,7 +1132,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, > static bool > -CopyReadLine(CopyFromState cstate) > +CopyReadLine(CopyFromState cstate, bool is_csv) > @@ -1163,7 +1208,7 @@ CopyReadLine(CopyFromState cstate) > static bool > -CopyReadLineText(CopyFromState cstate) > +CopyReadLineText(CopyFromState cstate, bool is_csv) We may want to add a comment why we don't use "inline" nor "pg_attribute_always_inline" here: https://www.postgresql.org/message-id/CAD21AoBNfKDbJnu-zONNpG820ZXYC0fuTSLrJ-UdRqU4qp2wog%40mail.gma... > Yes, I'm not sure it's really necessary to make it inline since the > benchmark results don't show much difference. Probably this is because > the function has 'is_csv' in some 'if' branches but the compiler > cannot optimize out the whole 'if' branches as most 'if' branches > check 'is_csv' and other variables. Or we can add "inline" not "pg_attribute_always_inline" here as a hint for compiler. > --- a/src/include/commands/copyapi.h > +++ b/src/include/commands/copyapi.h > @@ -52,4 +52,50 @@ typedef struct CopyToRoutine > + /* End a COPY FROM. This callback is called once at the end of COPY FROM */ The last "." is missing: ... COPY FROM. I think that these patches are ready to push too. Thanks, -- kou ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Make COPY format extendable: Extract COPY TO format implementations 2025-02-07 13:01 Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-20 23:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 2025-02-21 02:48 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-25 22:05 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 2025-02-25 23:51 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> @ 2025-02-26 01:14 ` Masahiko Sawada <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Masahiko Sawada @ 2025-02-26 01:14 UTC (permalink / raw) To: Sutou Kouhei <[email protected]>; +Cc: [email protected]; [email protected]; pgsql-hackers On Tue, Feb 25, 2025 at 3:52 PM Sutou Kouhei <[email protected]> wrote: > > Thank you for reviewing the patches. I've addressed comments except for the following comment: > > --- a/src/backend/commands/copyfromparse.c > > +++ b/src/backend/commands/copyfromparse.c > > > @@ -1087,7 +1132,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, > > > static bool > > -CopyReadLine(CopyFromState cstate) > > +CopyReadLine(CopyFromState cstate, bool is_csv) > > > @@ -1163,7 +1208,7 @@ CopyReadLine(CopyFromState cstate) > > > static bool > > -CopyReadLineText(CopyFromState cstate) > > +CopyReadLineText(CopyFromState cstate, bool is_csv) > > We may want to add a comment why we don't use "inline" nor > "pg_attribute_always_inline" here: > > https://www.postgresql.org/message-id/CAD21AoBNfKDbJnu-zONNpG820ZXYC0fuTSLrJ-UdRqU4qp2wog%40mail.gma... > > > Yes, I'm not sure it's really necessary to make it inline since the > > benchmark results don't show much difference. Probably this is because > > the function has 'is_csv' in some 'if' branches but the compiler > > cannot optimize out the whole 'if' branches as most 'if' branches > > check 'is_csv' and other variables. > > Or we can add "inline" not "pg_attribute_always_inline" here > as a hint for compiler. I think we should not add inline unless we see a performance improvement. Also, I find that it would be independent with this refactoring so we can add it later if needed. I've attached updated patches. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com Attachments: [application/octet-stream] v34-0002-Refactor-COPY-FROM-to-use-format-callback-functi.patch (32.7K, ../../CAD21AoB3TiyuCcu02itGktUE6L4YGqwWT_LRtYrFkW7xedoe+g@mail.gmail.com/2-v34-0002-Refactor-COPY-FROM-to-use-format-callback-functi.patch) download | inline diff: From 7be14abbee2f77b953af176221ac05ce74cdd590 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada <[email protected]> Date: Tue, 25 Feb 2025 13:58:58 -0800 Subject: [PATCH v34 2/2] Refactor COPY FROM to use format callback functions. This commit introduces a new CopyFromRoutine struct, which is a set of callback routines to read tuples in a specific format. It also makes COPY FROM with the existing formats (text, CSV, and binary) utilize these format callbacks. This change is a preliminary step towards making the COPY FROM command extensible in terms of output formats. Similar to XXXX, this refactoring contributes to a performance improvement by reducing the number of "if" branches that need to be checked on a per-row basis when sending field representations in text or CSV mode. The performance benchmark results showed ~5% performance gain in text or CSV mode. Author: Sutou Kouhei <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Andres Freund <[email protected]> Reviewed-by: Tomas Vondra <[email protected]> Reviewed-by: Junwang Zhao <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/copyfrom.c | 192 +++++++--- src/backend/commands/copyfromparse.c | 445 +++++++++++++---------- src/include/commands/copy.h | 2 - src/include/commands/copyapi.h | 50 ++- src/include/commands/copyfrom_internal.h | 11 + src/tools/pgindent/typedefs.list | 1 + 6 files changed, 457 insertions(+), 244 deletions(-) diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 8875d79d59a..4c2479974c4 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -28,7 +28,7 @@ #include "access/tableam.h" #include "access/xact.h" #include "catalog/namespace.h" -#include "commands/copy.h" +#include "commands/copyapi.h" #include "commands/copyfrom_internal.h" #include "commands/progress.h" #include "commands/trigger.h" @@ -106,6 +106,145 @@ typedef struct CopyMultiInsertInfo /* non-export function prototypes */ static void ClosePipeFromProgram(CopyFromState cstate); +/* + * Built-in format-specific routines. One-row callbacks are defined in + * copyfromparse.c + */ +static void CopyFromTextLikeInFunc(CopyFromState cstate, Oid atttypid, FmgrInfo *finfo, + Oid *typioparam); +static void CopyFromTextLikeStart(CopyFromState cstate, TupleDesc tupDesc); +static void CopyFromTextLikeEnd(CopyFromState cstate); +static void CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid, + FmgrInfo *finfo, Oid *typioparam); +static void CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc); +static void CopyFromBinaryEnd(CopyFromState cstate); + + +/* + * COPY FROM routines for built-in formats. + * + * CSV and text formats share the same TextLike routines except for the + * one-row callback. + */ + +/* text format */ +static const CopyFromRoutine CopyFromRoutineText = { + .CopyFromInFunc = CopyFromTextLikeInFunc, + .CopyFromStart = CopyFromTextLikeStart, + .CopyFromOneRow = CopyFromTextOneRow, + .CopyFromEnd = CopyFromTextLikeEnd, +}; + +/* CSV format */ +static const CopyFromRoutine CopyFromRoutineCSV = { + .CopyFromInFunc = CopyFromTextLikeInFunc, + .CopyFromStart = CopyFromTextLikeStart, + .CopyFromOneRow = CopyFromCSVOneRow, + .CopyFromEnd = CopyFromTextLikeEnd, +}; + +/* binary format */ +static const CopyFromRoutine CopyFromRoutineBinary = { + .CopyFromInFunc = CopyFromBinaryInFunc, + .CopyFromStart = CopyFromBinaryStart, + .CopyFromOneRow = CopyFromBinaryOneRow, + .CopyFromEnd = CopyFromBinaryEnd, +}; + +/* Return a COPY FROM routine for the given options */ +static const CopyFromRoutine * +CopyFromGetRoutine(CopyFormatOptions opts) +{ + if (opts.csv_mode) + return &CopyFromRoutineCSV; + else if (opts.binary) + return &CopyFromRoutineBinary; + + /* default is text */ + return &CopyFromRoutineText; +} + +/* Implementation of the start callback for text and CSV formats */ +static void +CopyFromTextLikeStart(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; used by CSV and text + * format. + */ + attr_count = list_length(cstate->attnumlist); + cstate->max_fields = attr_count; + cstate->raw_fields = (char **) palloc(attr_count * sizeof(char *)); +} + +/* + * Implementation of the infunc callback for text and CSV formats. Assign + * the input function data to the given *finfo. + */ +static void +CopyFromTextLikeInFunc(CopyFromState cstate, Oid atttypid, FmgrInfo *finfo, + Oid *typioparam) +{ + Oid func_oid; + + getTypeInputInfo(atttypid, &func_oid, typioparam); + fmgr_info(func_oid, finfo); +} + +/* Implementation of the end callback for text and CSV formats */ +static void +CopyFromTextLikeEnd(CopyFromState cstate) +{ + /* nothing to do */ +} + +/* Implementation of the start callback for binary format */ +static void +CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc) +{ + /* Read and verify binary header */ + ReceiveCopyBinaryHeader(cstate); +} + +/* + * Implementation of the infunc callback for binary format. Assign + * the binary input function to the given *finfo. + */ +static void +CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid, + FmgrInfo *finfo, Oid *typioparam) +{ + Oid func_oid; + + getTypeBinaryInputInfo(atttypid, &func_oid, typioparam); + fmgr_info(func_oid, finfo); +} + +/* Implementation of the end callback for binary format */ +static void +CopyFromBinaryEnd(CopyFromState cstate) +{ + /* nothing to do */ +} + /* * error context callback for COPY FROM * @@ -1403,7 +1542,6 @@ BeginCopyFrom(ParseState *pstate, num_defaults; FmgrInfo *in_functions; Oid *typioparams; - Oid in_func_oid; int *defmap; ExprState **defexprs; MemoryContext oldcontext; @@ -1435,6 +1573,9 @@ BeginCopyFrom(ParseState *pstate, /* Extract options from the statement node tree */ ProcessCopyOptions(pstate, &cstate->opts, true /* is_from */ , options); + /* Set the format routine */ + cstate->routine = CopyFromGetRoutine(cstate->opts); + /* Process the target relation */ cstate->rel = rel; @@ -1590,25 +1731,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. */ @@ -1641,13 +1763,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; @@ -1782,20 +1900,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 *)); - } + cstate->routine->CopyFromStart(cstate, tupDesc); MemoryContextSwitchTo(oldcontext); @@ -1808,6 +1913,9 @@ BeginCopyFrom(ParseState *pstate, void EndCopyFrom(CopyFromState cstate) { + /* Invoke the end callback */ + 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 caccdc8563c..7b14ef699a9 100644 --- a/src/backend/commands/copyfromparse.c +++ b/src/backend/commands/copyfromparse.c @@ -62,7 +62,7 @@ #include <unistd.h> #include <sys/stat.h> -#include "commands/copy.h" +#include "commands/copyapi.h" #include "commands/copyfrom_internal.h" #include "commands/progress.h" #include "executor/executor.h" @@ -140,8 +140,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 bool CopyReadLine(CopyFromState cstate, bool is_csv); +static bool CopyReadLineText(CopyFromState cstate, bool is_csv); static int CopyReadAttributesText(CopyFromState cstate); static int CopyReadAttributesCSV(CopyFromState cstate); static Datum CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo, @@ -740,9 +740,12 @@ CopyReadBinaryData(CopyFromState cstate, char *dest, int nbytes) * in the relation. * * NOTE: force_not_null option are not applied to the returned fields. + * + * We use pg_attribute_always_inline to reduce function call overhead + * and to help compilers to optimize away the 'is_csv' condition. */ -bool -NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) +static pg_attribute_always_inline bool +NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields, bool is_csv) { int fldct; bool done; @@ -759,13 +762,13 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) tupDesc = RelationGetDescr(cstate->rel); cstate->cur_lineno++; - done = CopyReadLine(cstate); + done = CopyReadLine(cstate, is_csv); if (cstate->opts.header_line == COPY_HEADER_MATCH) { int fldnum; - if (cstate->opts.csv_mode) + if (is_csv) fldct = CopyReadAttributesCSV(cstate); else fldct = CopyReadAttributesText(cstate); @@ -809,7 +812,7 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) cstate->cur_lineno++; /* Actually read the line into memory here */ - done = CopyReadLine(cstate); + done = CopyReadLine(cstate, is_csv); /* * EOF at start of line means we're done. If we see EOF after some @@ -819,8 +822,10 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) if (done && cstate->line_buf.len == 0) return false; - /* Parse the line into de-escaped field values */ - if (cstate->opts.csv_mode) + /* + * Parse the line into de-escaped field values. + */ + if (is_csv) fldct = CopyReadAttributesCSV(cstate); else fldct = CopyReadAttributesText(cstate); @@ -847,233 +852,275 @@ 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) + /* Get one row from source */ + if (!cstate->routine->CopyFromOneRow(cstate, econtext, values, nulls)) + return false; + + /* + * Now compute and insert any defaults available for the columns not + * provided by the input data. Anything not processed here or above will + * remain NULL. + */ + for (i = 0; i < num_defaults; i++) { - char **field_strings; - ListCell *cur; - int fldct; - int fieldno; - char *string; + /* + * 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); - /* read raw fields in the next line */ - if (!NextCopyFromRawFields(cstate, &field_strings, &fldct)) - return false; + values[defmap[i]] = ExecEvalExpr(defexprs[defmap[i]], econtext, + &nulls[defmap[i]]); + } + + return true; +} + +/* Implementation of the per-row callback for text format */ +bool +CopyFromTextOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values, + bool *nulls) +{ + return CopyFromTextLikeOneRow(cstate, econtext, values, nulls, false); +} + +/* Implementation of the per-row callback for CSV format */ +bool +CopyFromCSVOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values, + bool *nulls) +{ + return CopyFromTextLikeOneRow(cstate, econtext, values, nulls, true); +} + +/* + * Workhorse for CopyFromTextOneRow() and CopyFromCSVOneRow(). + * + * We use pg_attribute_always_inline to reduce function call overhead + * and to help compilers to optimize away the 'is_csv' condition. + */ +static pg_attribute_always_inline bool +CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext, + Datum *values, bool *nulls, bool is_csv) +{ + 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, is_csv)) + 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); - /* check for overflowing fields */ - if (attr_count > 0 && fldct > attr_count) + if (fieldno >= fldct) ereport(ERROR, (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), - errmsg("extra data after last expected column"))); - - fieldno = 0; + errmsg("missing data for column \"%s\"", + NameStr(att->attname)))); + string = field_strings[fieldno++]; - /* Loop to read the user attributes on the line. */ - foreach(cur, cstate->attnumlist) + if (cstate->convert_select_flags && + !cstate->convert_select_flags[m]) { - 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++]; + /* ignore input field, leaving column as NULL */ + continue; + } - if (cstate->convert_select_flags && - !cstate->convert_select_flags[m]) + if (is_csv) + { + if (string == NULL && + cstate->opts.force_notnull_flags[m]) { - /* ignore input field, leaving column as NULL */ - continue; + /* + * FORCE_NOT_NULL option is set and column is NULL - convert + * it to the NULL string. + */ + string = cstate->opts.null_print; } - - if (cstate->opts.csv_mode) + else if (string != NULL && cstate->opts.force_null_flags[m] + && strcmp(string, cstate->opts.null_print) == 0) { - 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; - } + /* + * 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; + cstate->cur_attname = NameStr(att->attname); + cstate->cur_attval = string; - if (string != NULL) - nulls[m] = false; + 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); + if (cstate->defaults[m]) + { + /* We must have switched into the per-tuple memory context */ + Assert(econtext != NULL); + Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory); - values[m] = ExecEvalExpr(defexprs[m], econtext, &nulls[m]); - } + 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])) - { - Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP); + /* + * 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])) + { + Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP); - cstate->num_errors++; + cstate->num_errors++; - if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE) - { - /* - * Since we emit line number and column info in the below - * notice message, we suppress error context information - * other than the relation name. - */ - Assert(!cstate->relname_only); - cstate->relname_only = true; + if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE) + { + /* + * Since we emit line number and column info in the below + * notice message, we suppress error context information other + * than the relation name. + */ + Assert(!cstate->relname_only); + cstate->relname_only = true; - if (cstate->cur_attval) - { - char *attval; - - attval = CopyLimitPrintoutLength(cstate->cur_attval); - ereport(NOTICE, - errmsg("skipping row due to data type incompatibility at line %llu for column \"%s\": \"%s\"", - (unsigned long long) cstate->cur_lineno, - cstate->cur_attname, - attval)); - pfree(attval); - } - else - ereport(NOTICE, - errmsg("skipping row due to data type incompatibility at line %llu for column \"%s\": null input", - (unsigned long long) cstate->cur_lineno, - cstate->cur_attname)); - - /* reset relname_only */ - cstate->relname_only = false; + if (cstate->cur_attval) + { + char *attval; + + attval = CopyLimitPrintoutLength(cstate->cur_attval); + ereport(NOTICE, + errmsg("skipping row due to data type incompatibility at line %llu for column \"%s\": \"%s\"", + (unsigned long long) cstate->cur_lineno, + cstate->cur_attname, + attval)); + pfree(attval); } + else + ereport(NOTICE, + errmsg("skipping row due to data type incompatibility at line %llu for column \"%s\": null input", + (unsigned long long) cstate->cur_lineno, + cstate->cur_attname)); - return true; + /* reset relname_only */ + cstate->relname_only = false; } - cstate->cur_attname = NULL; - cstate->cur_attval = NULL; + return true; } - Assert(fieldno == attr_count); + cstate->cur_attname = NULL; + cstate->cur_attval = NULL; } - else - { - /* binary */ - int16 fld_count; - ListCell *cur; - cstate->cur_lineno++; + Assert(fieldno == attr_count); - if (!CopyGetInt16(cstate, &fld_count)) - { - /* EOF detected (end of file, or protocol-level EOF) */ - return false; - } + return true; +} - 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; +/* Implementation of the per-row callback for 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; - if (CopyReadBinaryData(cstate, &dummy, 1) > 0) - ereport(ERROR, - (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), - errmsg("received copy data after EOF marker"))); - return false; - } + tupDesc = RelationGetDescr(cstate->rel); + attr_count = list_length(cstate->attnumlist); - 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))); + cstate->cur_lineno++; - 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 (!CopyGetInt16(cstate, &fld_count)) + { + /* EOF detected (end of file, or protocol-level EOF) */ + return false; } - /* - * Now compute and insert any defaults available for the columns not - * provided by the input data. Anything not processed here or above will - * remain NULL. - */ - for (i = 0; i < num_defaults; i++) + if (fld_count == -1) { /* - * The caller must supply econtext and have switched into the - * per-tuple memory context in it. + * 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. */ - Assert(econtext != NULL); - Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory); + char dummy; - values[defmap[i]] = ExecEvalExpr(defexprs[defmap[i]], econtext, - &nulls[defmap[i]]); + 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; @@ -1087,7 +1134,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, * in the final value of line_buf. */ static bool -CopyReadLine(CopyFromState cstate) +CopyReadLine(CopyFromState cstate, bool is_csv) { bool result; @@ -1095,7 +1142,7 @@ CopyReadLine(CopyFromState cstate) cstate->line_buf_valid = false; /* Parse data and transfer into line_buf */ - result = CopyReadLineText(cstate); + result = CopyReadLineText(cstate, is_csv); if (result) { @@ -1163,7 +1210,7 @@ CopyReadLine(CopyFromState cstate) * CopyReadLineText - inner loop of CopyReadLine for text mode */ static bool -CopyReadLineText(CopyFromState cstate) +CopyReadLineText(CopyFromState cstate, bool is_csv) { char *copy_input_buf; int input_buf_ptr; @@ -1178,7 +1225,7 @@ CopyReadLineText(CopyFromState cstate) char quotec = '\0'; char escapec = '\0'; - if (cstate->opts.csv_mode) + if (is_csv) { quotec = cstate->opts.quote[0]; escapec = cstate->opts.escape[0]; @@ -1255,7 +1302,7 @@ CopyReadLineText(CopyFromState cstate) prev_raw_ptr = input_buf_ptr; c = copy_input_buf[input_buf_ptr++]; - if (cstate->opts.csv_mode) + if (is_csv) { /* * If character is '\r', we may need to look ahead below. Force @@ -1294,7 +1341,7 @@ CopyReadLineText(CopyFromState cstate) } /* Process \r */ - if (c == '\r' && (!cstate->opts.csv_mode || !in_quote)) + if (c == '\r' && (!is_csv || !in_quote)) { /* Check for \r\n on first line, _and_ handle \r\n. */ if (cstate->eol_type == EOL_UNKNOWN || @@ -1322,10 +1369,10 @@ CopyReadLineText(CopyFromState cstate) if (cstate->eol_type == EOL_CRNL) ereport(ERROR, (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), - !cstate->opts.csv_mode ? + !is_csv ? errmsg("literal carriage return found in data") : errmsg("unquoted carriage return found in data"), - !cstate->opts.csv_mode ? + !is_csv ? errhint("Use \"\\r\" to represent carriage return.") : errhint("Use quoted CSV field to represent carriage return."))); @@ -1339,10 +1386,10 @@ CopyReadLineText(CopyFromState cstate) else if (cstate->eol_type == EOL_NL) ereport(ERROR, (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), - !cstate->opts.csv_mode ? + !is_csv ? errmsg("literal carriage return found in data") : errmsg("unquoted carriage return found in data"), - !cstate->opts.csv_mode ? + !is_csv ? 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 */ @@ -1350,15 +1397,15 @@ CopyReadLineText(CopyFromState cstate) } /* Process \n */ - if (c == '\n' && (!cstate->opts.csv_mode || !in_quote)) + if (c == '\n' && (!is_csv || !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 ? + !is_csv ? errmsg("literal newline found in data") : errmsg("unquoted newline found in data"), - !cstate->opts.csv_mode ? + !is_csv ? errhint("Use \"\\n\" to represent newline.") : errhint("Use quoted CSV field to represent newline."))); cstate->eol_type = EOL_NL; /* in case not set yet */ @@ -1370,7 +1417,7 @@ CopyReadLineText(CopyFromState cstate) * Process backslash, except in CSV mode where backslash is a normal * character. */ - if (c == '\\' && !cstate->opts.csv_mode) + if (c == '\\' && !is_csv) { char c2; diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h index 06dfdfef721..7bc044e2816 100644 --- a/src/include/commands/copy.h +++ b/src/include/commands/copy.h @@ -107,8 +107,6 @@ extern CopyFromState BeginCopyFrom(ParseState *pstate, Relation rel, Node *where extern void EndCopyFrom(CopyFromState cstate); extern bool NextCopyFrom(CopyFromState cstate, ExprContext *econtext, Datum *values, bool *nulls); -extern bool NextCopyFromRawFields(CopyFromState cstate, - char ***fields, int *nfields); extern void CopyFromErrorCallback(void *arg); extern char *CopyLimitPrintoutLength(const char *str); diff --git a/src/include/commands/copyapi.h b/src/include/commands/copyapi.h index bd2d386816e..2a2d2f9876b 100644 --- a/src/include/commands/copyapi.h +++ b/src/include/commands/copyapi.h @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * copyapi.h - * API for COPY TO handlers + * API for COPY TO/FROM handlers * * * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group @@ -54,4 +54,52 @@ typedef struct CopyToRoutine void (*CopyToEnd) (CopyToState cstate); } CopyToRoutine; +/* + * 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 +{ + /* + * Set input function information. This callback is called once at the + * beginning of COPY FROM. + * + * '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); + + /* + * Start a COPY FROM. This callback is called once at the beginning of + * COPY FROM. + * + * '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); + + /* + * Read one row from the source and fill *values and *nulls. + * + * '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 read. + */ + bool (*CopyFromOneRow) (CopyFromState cstate, ExprContext *econtext, + Datum *values, bool *nulls); + + /* + * End a COPY FROM. This callback is called once at the end of COPY FROM. + */ + void (*CopyFromEnd) (CopyFromState cstate); +} CopyFromRoutine; + #endif /* COPYAPI_H */ diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h index 1d8ac8f62e6..c8b22af22d8 100644 --- a/src/include/commands/copyfrom_internal.h +++ b/src/include/commands/copyfrom_internal.h @@ -58,6 +58,9 @@ typedef enum CopyInsertMethod */ typedef struct CopyFromStateData { + /* format routine */ + const struct CopyFromRoutine *routine; + /* low-level state data */ CopySource copy_src; /* type of copy source */ FILE *copy_file; /* used if copy_src == COPY_FILE */ @@ -183,4 +186,12 @@ typedef struct CopyFromStateData extern void ReceiveCopyBegin(CopyFromState cstate); extern void ReceiveCopyBinaryHeader(CopyFromState cstate); +/* One-row callbacks for built-in formats defined in copyfromparse.c */ +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); + #endif /* COPYFROM_INTERNAL_H */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index b0c97b0fb85..eca93ab9553 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -501,6 +501,7 @@ ConvertRowtypeExpr CookedConstraint CopyDest CopyFormatOptions +CopyFromRoutine CopyFromState CopyFromStateData CopyHeaderChoice -- 2.43.5 [application/octet-stream] v34-0001-Refactor-COPY-TO-to-use-format-callback-function.patch (18.0K, ../../CAD21AoB3TiyuCcu02itGktUE6L4YGqwWT_LRtYrFkW7xedoe+g@mail.gmail.com/3-v34-0001-Refactor-COPY-TO-to-use-format-callback-function.patch) download | inline diff: From 4a95959a25680474fced6fab892d8a43b1166b57 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada <[email protected]> Date: Tue, 25 Feb 2025 13:58:41 -0800 Subject: [PATCH v34 1/2] Refactor COPY TO to use format callback functions. This commit introduces a new CopyToRoutine struct, which is a set of callback routines to copy tuples in a specific format. It also makes the existing formats (text, CSV, and binary) utilize these format callbacks. This change is a preliminary step towards making the COPY TO command extensible in terms of output formats. Additionally, this refactoring contributes to a performance improvement by reducing the number of "if" branches that need to be checked on a per-row basis when sending field representations in text or CSV mode. The performance benchmark results showed ~5% performance gain in text or CSV mode. Author: Sutou Kouhei <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Andres Freund <[email protected]> Reviewed-by: Tomas Vondra <[email protected]> Reviewed-by: Junwang Zhao <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/commands/copyto.c | 438 +++++++++++++++++++++---------- src/include/commands/copyapi.h | 57 ++++ src/tools/pgindent/typedefs.list | 1 + 3 files changed, 355 insertions(+), 141 deletions(-) create mode 100644 src/include/commands/copyapi.h diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 091fbc12cc5..721d29f8e53 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -19,7 +19,7 @@ #include <sys/stat.h> #include "access/tableam.h" -#include "commands/copy.h" +#include "commands/copyapi.h" #include "commands/progress.h" #include "executor/execdesc.h" #include "executor/executor.h" @@ -64,6 +64,9 @@ typedef enum CopyDest */ typedef struct CopyToStateData { + /* format-specific routines */ + const CopyToRoutine *routine; + /* low-level state data */ CopyDest copy_dest; /* type of copy source/destination */ FILE *copy_file; /* used if copy_dest == COPY_FILE */ @@ -114,6 +117,19 @@ static void CopyAttributeOutText(CopyToState cstate, const char *string); static void CopyAttributeOutCSV(CopyToState cstate, const char *string, bool use_quote); +/* built-in format-specific routines */ +static void CopyToTextLikeStart(CopyToState cstate, TupleDesc tupDesc); +static void CopyToTextLikeOutFunc(CopyToState cstate, Oid atttypid, FmgrInfo *finfo); +static void CopyToTextOneRow(CopyToState cstate, TupleTableSlot *slot); +static void CopyToCSVOneRow(CopyToState cstate, TupleTableSlot *slot); +static void CopyToTextLikeOneRow(CopyToState cstate, TupleTableSlot *slot, + bool is_csv); +static void CopyToTextLikeEnd(CopyToState cstate); +static void CopyToBinaryStart(CopyToState cstate, TupleDesc tupDesc); +static void CopyToBinaryOutFunc(CopyToState cstate, Oid atttypid, FmgrInfo *finfo); +static void CopyToBinaryOneRow(CopyToState cstate, TupleTableSlot *slot); +static void CopyToBinaryEnd(CopyToState cstate); + /* Low-level communications functions */ static void SendCopyBegin(CopyToState cstate); static void SendCopyEnd(CopyToState cstate); @@ -121,9 +137,251 @@ static void CopySendData(CopyToState cstate, const void *databuf, int datasize); static void CopySendString(CopyToState cstate, const char *str); static void CopySendChar(CopyToState cstate, char c); static void CopySendEndOfRow(CopyToState cstate); +static void CopySendTextLikeEndOfRow(CopyToState cstate); static void CopySendInt32(CopyToState cstate, int32 val); static void CopySendInt16(CopyToState cstate, int16 val); +/* + * COPY TO routines for built-in formats. + * + * CSV and text formats share the same TextLike routines except for the + * one-row callback. + */ + +/* text format */ +static const CopyToRoutine CopyToRoutineText = { + .CopyToStart = CopyToTextLikeStart, + .CopyToOutFunc = CopyToTextLikeOutFunc, + .CopyToOneRow = CopyToTextOneRow, + .CopyToEnd = CopyToTextLikeEnd, +}; + +/* CSV format */ +static const CopyToRoutine CopyToRoutineCSV = { + .CopyToStart = CopyToTextLikeStart, + .CopyToOutFunc = CopyToTextLikeOutFunc, + .CopyToOneRow = CopyToCSVOneRow, + .CopyToEnd = CopyToTextLikeEnd, +}; + +/* binary format */ +static const CopyToRoutine CopyToRoutineBinary = { + .CopyToStart = CopyToBinaryStart, + .CopyToOutFunc = CopyToBinaryOutFunc, + .CopyToOneRow = CopyToBinaryOneRow, + .CopyToEnd = CopyToBinaryEnd, +}; + +/* Return a COPY TO routine for the given options */ +static const CopyToRoutine * +CopyToGetRoutine(CopyFormatOptions opts) +{ + if (opts.csv_mode) + return &CopyToRoutineCSV; + else if (opts.binary) + return &CopyToRoutineBinary; + + /* default is text */ + return &CopyToRoutineText; +} + +/* Implementation of the start callback for text and CSV formats */ +static void +CopyToTextLikeStart(CopyToState cstate, TupleDesc tupDesc) +{ + /* + * 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) + { + ListCell *cur; + 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); + } + + CopySendTextLikeEndOfRow(cstate); + } +} + +/* + * Implementation of the outfunc callback for text and CSV formats. Assign + * the output function data to the given *finfo. + */ +static void +CopyToTextLikeOutFunc(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); +} + +/* Implementation of the per-row callback for text format */ +static void +CopyToTextOneRow(CopyToState cstate, TupleTableSlot *slot) +{ + CopyToTextLikeOneRow(cstate, slot, false); +} + +/* Implementation of the per-row callback for CSV format */ +static void +CopyToCSVOneRow(CopyToState cstate, TupleTableSlot *slot) +{ + CopyToTextLikeOneRow(cstate, slot, true); +} + +/* + * Workhorse for CopyToTextOneRow() and CopyToCSVOneRow(). + * + * We use pg_attribute_always_inline to reduce function call overhead + * and to help compilers to optimize away the 'is_csv' condition. + */ +static pg_attribute_always_inline void +CopyToTextLikeOneRow(CopyToState cstate, + TupleTableSlot *slot, + bool is_csv) +{ + bool need_delim = false; + FmgrInfo *out_functions = cstate->out_functions; + + foreach_int(attnum, cstate->attnumlist) + { + 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 (is_csv) + CopyAttributeOutCSV(cstate, string, + cstate->opts.force_quote_flags[attnum - 1]); + else + CopyAttributeOutText(cstate, string); + } + } + + CopySendTextLikeEndOfRow(cstate); +} + +/* Implementation of the end callback for text and CSV formats */ +static void +CopyToTextLikeEnd(CopyToState cstate) +{ + /* Nothing to do here */ +} + +/* + * Implementation of the start callback for binary format. Send a header + * for a binary copy. + */ +static void +CopyToBinaryStart(CopyToState cstate, TupleDesc tupDesc) +{ + int32 tmp; + + /* Signature */ + CopySendData(cstate, BinarySignature, 11); + /* Flags field */ + tmp = 0; + CopySendInt32(cstate, tmp); + /* No header extension */ + tmp = 0; + CopySendInt32(cstate, tmp); +} + +/* + * Implementation of the outfunc callback for binary format. Assign + * the binary output function to the given *finfo. + */ +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); +} + +/* Implementation of the per-row callback for binary format */ +static void +CopyToBinaryOneRow(CopyToState cstate, TupleTableSlot *slot) +{ + FmgrInfo *out_functions = cstate->out_functions; + + /* Binary per-tuple header */ + CopySendInt16(cstate, list_length(cstate->attnumlist)); + + foreach_int(attnum, cstate->attnumlist) + { + 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); +} + +/* Implementation of the end callback 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); +} /* * Send copy start/stop messages for frontend copies. These have changed @@ -191,16 +449,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)) @@ -235,10 +483,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; @@ -254,6 +498,35 @@ CopySendEndOfRow(CopyToState cstate) resetStringInfo(fe_msgbuf); } +/* + * Wrapper function of CopySendEndOfRow for text and CSV formats. Sends the + * line termination and do common appropriate things for the end of row. + */ +static inline void +CopySendTextLikeEndOfRow(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); +} + /* * These functions do apply some data conversion */ @@ -426,6 +699,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 +1048,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 +1064,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 +1103,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); @@ -904,74 +1116,18 @@ DoCopyTo(CopyToState cstate) /* * Emit one row during DoCopyTo(). */ -static void +static inline void CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) { - FmgrInfo *out_functions = cstate->out_functions; MemoryContext oldcontext; 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); - if (!cstate->opts.binary) - { - bool need_delim = false; - - foreach_int(attnum, cstate->attnumlist) - { - Datum value = slot->tts_values[attnum - 1]; - bool isnull = slot->tts_isnull[attnum - 1]; - char *string; - - if (need_delim) - CopySendChar(cstate, cstate->opts.delim[0]); - need_delim = true; - - if (isnull) - CopySendString(cstate, cstate->opts.null_print_client); - else - { - 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 - { - foreach_int(attnum, cstate->attnumlist) - { - Datum value = slot->tts_values[attnum - 1]; - bool isnull = slot->tts_isnull[attnum - 1]; - bytea *outputbytes; - - if (isnull) - CopySendInt32(cstate, -1); - else - { - 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/include/commands/copyapi.h b/src/include/commands/copyapi.h new file mode 100644 index 00000000000..bd2d386816e --- /dev/null +++ b/src/include/commands/copyapi.h @@ -0,0 +1,57 @@ +/*------------------------------------------------------------------------- + * + * copyapi.h + * API for COPY TO handlers + * + * + * Portions Copyright (c) 1996-2025, 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 "commands/copy.h" + +/* + * 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 +{ + /* + * Set output function information. This callback is called once at the + * beginning of COPY TO. + * + * 'finfo' can be optionally filled to provide the catalog information of + * the output function. + * + * 'atttypid' is the OID of data type used by the relation's attribute. + */ + void (*CopyToOutFunc) (CopyToState cstate, Oid atttypid, + FmgrInfo *finfo); + + /* + * Start a COPY TO. This callback is called once at the beginning of COPY + * TO. + * + * 'tupDesc' is the tuple descriptor of the relation from where the data + * is read. + */ + void (*CopyToStart) (CopyToState cstate, TupleDesc tupDesc); + + /* + * Write one row stored in 'slot' to the destination. + */ + void (*CopyToOneRow) (CopyToState cstate, TupleTableSlot *slot); + + /* + * End a COPY TO. This callback is called once at the end of COPY TO. + */ + void (*CopyToEnd) (CopyToState cstate); +} CopyToRoutine; + +#endif /* COPYAPI_H */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 1649f5e1011..b0c97b0fb85 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -512,6 +512,7 @@ CopyMultiInsertInfo CopyOnErrorChoice CopySource CopyStmt +CopyToRoutine CopyToState CopyToStateData Cost -- 2.43.5 ^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2025-02-26 01:14 UTC | newest] Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2023-02-23 22:27 [PATCH v8 1/2] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2023-02-23 22:27 [PATCH v10 1/2] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2023-02-23 22:27 [PATCH v13 1/7] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2023-02-23 22:27 [PATCH v15 1/7] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2023-02-23 22:27 [PATCH v7 1/2] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2023-02-23 22:27 [PATCH v9 1/2] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2023-02-23 22:27 [PATCH v14 1/7] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2023-02-23 22:27 [PATCH v6 1/2] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2023-02-23 22:27 [PATCH v11 1/2] Move extra code out of the Pre/PostRestoreCommand() block. Nathan Bossart <[email protected]> 2025-02-07 13:01 Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-20 23:28 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 2025-02-21 02:48 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-25 22:05 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[email protected]> 2025-02-25 23:51 ` Re: Make COPY format extendable: Extract COPY TO format implementations Sutou Kouhei <[email protected]> 2025-02-26 01:14 ` Re: Make COPY format extendable: Extract COPY TO format implementations Masahiko Sawada <[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