Re: Use COPY for populating all pgbench tables

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Tristan Partin <tristan(at)neon(dot)tech>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Use COPY for populating all pgbench tables
Date: 2023-07-11 05:03:15
Message-ID: ZKzik5uamTwRWHsY@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jun 14, 2023 at 10:58:06AM -0500, Tristan Partin wrote:
> Again, I forget to actually attach. Holy guacamole.

Looks rather OK seen from here, applied 0001 while browsing the
series. I have a few comments about 0002.

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);
+}

I was looking at that, and while this strikes me as a duplication for
the second and third ones, I'm OK with the use of a callback to fill
in the data, even if naccounts and ntellers are equivalent to the
"base" number given to initPopulateTable(), because the "filler"
column has different expectations for each table. These routines
don't need a PGconn argument, by the way.

/* 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?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2023-07-11 05:17:04 RE: [Patch] Use *other* indexes on the subscriber when REPLICA IDENTITY is FULL
Previous Message Drouvot, Bertrand 2023-07-11 04:54:23 Re: Autogenerate some wait events code and documentation