Hi Abhijit, Thanks for your input. Please accept my updated patch.
Ciao, Gabriele On Tue, 16 Jan 2024 at 12:53, Abhijit Menon-Sen <a...@toroid.org> wrote: > At 2023-11-30 11:29:15 +0100, gabriele.bartol...@enterprisedb.com 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 <gabriele.bartol...@enterprisedb.com> > > 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 > -- Gabriele Bartolini Vice President, Cloud Native at EDB enterprisedb.com
v2-0001-Include-pgbench_history-in-partitioning-method-fo.patch
Description: Binary data