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
