Re: Use COPY for populating all pgbench tables

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)