public inbox for [email protected]  
help / color / mirror / Atom feed
From: Abhijit Menon-Sen <[email protected]>
To: Gabriele Bartolini <[email protected]>
Cc: [email protected]
Subject: Re: Extend pgbench partitioning to pgbench_history
Date: Tue, 16 Jan 2024 17:23:21 +0530
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+VUV5q9=N64fduq660bvFGQeFQcfqU2xh+XR_1OaPj87io3sg@mail.gmail.com>
References: <CA+VUV5q9=N64fduq660bvFGQeFQcfqU2xh+XR_1OaPj87io3sg@mail.gmail.com>

At 2023-11-30 11:29:15 +0100, [email protected] wrote:
>
> With the attached patch, I extend the partitioning capability to the
> pgbench_history table too.
> 
> I have been thinking of adding an option to control this, but I preferred
> to ask in this list whether it really makes sense or not (I struggle indeed
> to see use cases where accounts is partitioned and history is not).

I don't have a strong opinion about this, but I also can't think of a
reason to want to create partitions for pgbench_accounts but leave out
pgbench_history.

> From ba8f507b126a9c5bd22dd40bb8ce0c1f0c43ac59 Mon Sep 17 00:00:00 2001
> From: Gabriele Bartolini <[email protected]>
> Date: Thu, 30 Nov 2023 11:02:39 +0100
> Subject: [PATCH] Include pgbench_history in partitioning method for pgbench
> 
> In case partitioning, make sure that pgbench_history is also partitioned with
> the same criteria.

I think "If partitioning" or "If we're creating partitions" would read
better here. Also, same criteria as what? Maybe you could just add "as
pgbench_accounts" to the end.

> diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
> index 05d3f81619..4c02d2a61d 100644
> --- a/doc/src/sgml/ref/pgbench.sgml
> +++ b/doc/src/sgml/ref/pgbench.sgml
> […]
> @@ -378,9 +378,9 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
>        <term><option>--partitions=<replaceable>NUM</replaceable></option></term>
>        <listitem>
>         <para>
> -        Create a partitioned <literal>pgbench_accounts</literal> table with
> -        <replaceable>NUM</replaceable> partitions of nearly equal size for
> -        the scaled number of accounts.
> +        Create partitioned <literal>pgbench_accounts</literal> and <literal>pgbench_history</literal>
> +        tables with <replaceable>NUM</replaceable> partitions of nearly equal size for
> +        the scaled number of accounts - and future history records.
>          Default is <literal>0</literal>, meaning no partitioning.
>         </para>

I would just leave out the "-" and write "number of accounts and future
history records".

> diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
> index 2e1650d0ad..87adaf4d8f 100644
> --- a/src/bin/pgbench/pgbench.c
> +++ b/src/bin/pgbench/pgbench.c
> […]
> @@ -889,8 +889,10 @@ usage(void)
>  		   "  --index-tablespace=TABLESPACE\n"
>  		   "                           create indexes in the specified tablespace\n"
>  		   "  --partition-method=(range|hash)\n"
> -		   "                           partition pgbench_accounts with this method (default: range)\n"
> -		   "  --partitions=NUM         partition pgbench_accounts into NUM parts (default: 0)\n"
> +		   "                           partition pgbench_accounts and pgbench_history with this method"
> +		   "                           (default: range)."
> +		   "  --partitions=NUM         partition pgbench_accounts and pgbench_history into NUM parts"
> +		   "                           (default: 0)\n"
>  		   "  --tablespace=TABLESPACE  create tables in the specified tablespace\n"
>  		   "  --unlogged-tables        create tables as unlogged tables\n"
>  		   "\nOptions to select what to run:\n"

There's a missing newline after "(default: range).".

I read through the rest of the patch closely. It looks fine to me. It
applies, builds, and does create the partitions as intended.

-- Abhijit






view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Extend pgbench partitioning to pgbench_history
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox