Subject: Re: pgbench with partitioned tables

From: Sergey Tatarintsev <[email-masked]>
Date: Mon, 3 Feb 2025 19:23:16 +0700

03.02.2025 14:57, Álvaro Herrera пишет:
> On 2025-Feb-03, Sergey Tatarintsev wrote:
>
>> Thanks for the note. I changed the query in the patch (v2 patch attached)
>>
>> Btw, an additional benefit from the patch is that we can use foreign tables
>> (for example, to test postgres_fdw optimizations)
> Good thought, and maybe it would be better if the new function was
> "get_table_relkind" which just returns the char, and this check
>
>> +	/* Use COPY with FREEZE on v14 and later for all regular tables */
>> +	if ((PQserverVersion(con) >= 140000) && is_regular_table(con, table))
>>   			copy_statement_fmt = "copy %s from stdin with (freeze on)";
> can be "&& get_table_relkind(con, table) == RELKIND_RELATION"
>
> which I think is more natural.
>
sounds reasonable.

patch v3 attached

-- 
With best regards,
Sergey Tatarintsev,
PostgresPro
From cdda0dcb2c0fd8d14cddd185020fd0b1ecea59be Mon Sep 17 00:00:00 2001
From: Sergey Tatarintsev <[email-masked]>
Date: Mon, 3 Feb 2025 19:18:06 +0700
Subject: [PATCH] [PATCH-v3] Fix pgbench client-side data generation for
 partitioned tables

Client-side data generation cannot perform COPY WITH (FREEZE = ON) on partitioned
tables except pgbench_accounts.
Patch disables FREEZE for any non-regular tables
---
 src/bin/pgbench/pgbench.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 40592e6260..d003b407bf 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -53,6 +53,7 @@
 #include <sys/select.h>
 #endif
 
+#include "catalog/pg_class.h"
 #include "common/int.h"
 #include "common/logging.h"
 #include "common/pg_prng.h"
@@ -848,6 +849,29 @@ static const PsqlScanCallbacks pgbench_callbacks = {
 	NULL,						/* don't need get_variable functionality */
 };
 
+static char
+get_table_relkind(PGconn *con, const char *table)
+{
+	PGresult   *res;
+	char		*sql = psprintf("SELECT relkind FROM pg_catalog.pg_class WHERE oid='%s'::pg_catalog.regclass", table);
+	char		*val;
+	char		relkind = RELKIND_RELATION;
+
+	res = PQexec(con, sql);
+	if (PQresultStatus(res) != PGRES_TUPLES_OK)
+	{
+		pg_log_error("query failed: %s", PQerrorMessage(con));
+		pg_log_error_detail("Query was: %s", sql);
+		exit(1);
+	}
+	val = PQgetvalue(res, 0, 0);
+	Assert(strlen(val) == 1);
+	relkind = val[0];
+	PQclear(res);
+	pfree(sql);
+	return relkind;
+}
+
 static inline pg_time_usec_t
 pg_time_now(void)
 {
@@ -4962,16 +4986,10 @@ initPopulateTable(PGconn *con, const char *table, int64 base,
 
 	initPQExpBuffer(&sql);
 
-	/*
-	 * Use COPY with FREEZE on v14 and later for all the tables except
-	 * pgbench_accounts when it is partitioned.
-	 */
-	if (PQserverVersion(con) >= 140000)
-	{
-		if (strcmp(table, "pgbench_accounts") != 0 ||
-			partitions == 0)
+	/* Use COPY with FREEZE on v14 and later for all regular tables */
+	if ((PQserverVersion(con) >= 140000) && get_table_relkind(con, table) == RELKIND_RELATION)
 			copy_statement_fmt = "copy %s from stdin with (freeze on)";
-	}
+
 
 	n = pg_snprintf(copy_statement, sizeof(copy_statement), copy_statement_fmt, table);
 	if (n >= sizeof(copy_statement))
-- 
2.43.0

---

From: Melanie Plageman <[email-masked]>
Date: Fri, 7 Feb 2025 18:15:58 -0500

> > which I think is more natural.

Okay, I've stared at this a bit, and it seems basically fine the way
it is (I might add a bit more whitespace, clean up the commit message,
etc). So I'm interested in committing it. I will admit that having
never committed anything to pgbench, I'm a bit nervous. So, give me a
couple days to work up the nerve.

- Melanie