Re: Use COPY for populating all pgbench tables - Mailing list pgsql-hackers

From Tristan Partin
Subject Re: Use COPY for populating all pgbench tables
Date
Msg-id CTZFAQT1PDPQ.30VM4JHGNU3J@gonk
Whole thread Raw
In response to Re: Use COPY for populating all pgbench tables  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Use COPY for populating all pgbench tables
List pgsql-hackers
On Tue Jul 11, 2023 at 12:03 AM CDT, Michael Paquier wrote:
> On Wed, Jun 14, 2023 at 10:58:06AM -0500, Tristan Partin wrote:
>  static void
> -initGenerateDataClientSide(PGconn *con)
> +initBranch(PGconn *con, PQExpBufferData *sql, int64 curr)
> +{
> +       /* "filler" column defaults to NULL */
> +       printfPQExpBuffer(sql,
> +                                         INT64_FORMAT "\t0\t\n",
> +                                         curr + 1);
> +}
> +
> +static void
> +initTeller(PGconn *con, PQExpBufferData *sql, int64 curr)
> +{
> +       /* "filler" column defaults to NULL */
> +       printfPQExpBuffer(sql,
> +                                         INT64_FORMAT "\t" INT64_FORMAT "\t0\t\n",
> +                                         curr + 1, curr / ntellers + 1);
> +}
> +
> +static void
> +initAccount(PGconn *con, PQExpBufferData *sql, int64 curr)
> +{
> +       /* "filler" column defaults to blank padded empty string */
> +       printfPQExpBuffer(sql,
> +                                         INT64_FORMAT "\t" INT64_FORMAT "\t0\t\n",
> +                                         curr + 1, curr / naccounts + 1);
> +}
>
> These routines don't need a PGconn argument, by the way.

Nice catch!

>     /* use COPY with FREEZE on v14 and later without partitioning */
>     if (partitions == 0 && PQserverVersion(con) >= 140000)
> -       copy_statement = "copy pgbench_accounts from stdin with (freeze on)";
> +       copy_statement_fmt = "copy %s from stdin with (freeze on)";
>     else
> -       copy_statement = "copy pgbench_accounts from stdin";
> +       copy_statement_fmt = "copy %s from stdin";
>
> This seems a bit incorrect because partitioning only applies to
> pgbench_accounts, no?  This change means that the teller and branch
> tables would not benefit from FREEZE under a pgbench compiled with
> this patch and a backend version of 14 or newer if --partitions is
> used.  So, perhaps "partitions" should be an argument of
> initPopulateTable() specified for each table?

Whoops, looks like I didn't read the comment for what the partitions
variable means. It only applies to pgbench_accounts as you said. I don't
think passing it in as an argument would make much sense. Let me know
what you think about the change in this new version, which only hits the
first portion of the `if` statement if the table is pgbench_accounts.

--
Tristan Partin
Neon (https://neon.tech)

Attachment

pgsql-hackers by date:

Previous
From: Greg Sabino Mullane
Date:
Subject: Forgive trailing semicolons inside of config files
Next
From: "Tristan Partin"
Date:
Subject: Re: Make pgbench exit on SIGINT more reliably