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-21 02:14:57
Message-ID: ZLnqIQuMX8BzyFXR@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 20, 2023 at 02:22:51PM -0500, Tristan Partin wrote:
> Thanks for your testing Michael. I went ahead and added a test to make sure
> that this behavior doesn't regress accidentally, but I am struggling to get
> the test to fail using the previous version of this patch. Do you have any
> advice? This is my first time writing a test for Postgres. I can recreate
> the issue outside of the test script, but not within it for whatever reason.

We're all here to learn, and writing TAP tests is important these days
for a lot of patches.

+# Check that the pgbench_branches and pgbench_tellers filler columns are filled
+# with NULLs
+foreach my $table ('pgbench_branches', 'pgbench_tellers') {
+ my ($ret, $out, $err) = $node->psql(
+ 'postgres',
+ "SELECT COUNT(1) FROM $table;
+ SELECT COUNT(1) FROM $table WHERE filler is NULL;",
+ extra_params => ['--no-align', '--tuples-only']);
+
+ is($ret, 0, "psql $table counts status is 0");
+ is($err, '', "psql $table counts stderr is empty");
+ if ($out =~ m/^(\d+)\n(\d+)$/g) {
+ is($1, $2, "psql $table filler column filled with NULLs");
+ } else {
+ fail("psql $table stdout m/^(\\d+)\n(\\d+)$/g");
+ }
+}

This is IMO hard to parse, and I'd rather add some checks for the
accounts and history tables as well. Let's use four simple SQL
queries like what I posted upthread (no data for history inserted
after initialization), as of the attached. I'd be tempted to apply
that first as a separate patch, because we've never add coverage for
that and we have specific expectations in the code from this filler
column for tpc-b. And this needs to cover both client-side and
server-side data generation.

Note that the indentation was a bit incorrect, so fixed while on it.

Attached is a v7, with these tests (should be a patch on its own but
I'm lazy to split this morning) and some more adjustments that I have
done while going through the patch. What do you think?
--
Michael

Attachment Content-Type Size
v7-0001-Use-COPY-instead-of-INSERT-for-populating-all-tab.patch text/x-diff 11.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-07-21 03:08:47 Re: Support worker_spi to execute the function dynamically.
Previous Message David Rowley 2023-07-21 02:03:46 Re: Avoid stack frame setup in performance critical routines using tail calls