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