Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Wed, 12 Jul 2023 09:29:35 -0500
Message-Id: <CU09K8PHEOIZ.I5NPQDPOQ1SO@gonk>
Cc: <pgsql-hackers@postgresql.org>
Subject: Re: Use COPY for populating all pgbench tables
To: "Michael Paquier" <michael@paquier.xyz>
From: "Tristan Partin" <tristan@neon.tech>
References: <CSTU5P82ONZ1.19XFUGHMXHBRY@c3po>
 <CTCG9VOM6GJ4.3NDBRQ7U4EP4Z@gonk> <CTCHWOCZ9X8Z.1PDXVDUXZ3D0P@gonk>
 <ZKzik5uamTwRWHsY@paquier.xyz> <CTZFAQT1PDPQ.30VM4JHGNU3J@gonk>
 <ZK5C26HdJlRJ/yOl@paquier.xyz>
In-Reply-To: <ZK5C26HdJlRJ/yOl@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/CU09K8PHEOIZ.I5NPQDPOQ1SO%40gonk>
Precedence: bulk

On Wed Jul 12, 2023 at 1:06 AM CDT, Michael Paquier wrote:
> On Tue, Jul 11, 2023 at 09:46:43AM -0500, Tristan Partin wrote:
> > On Tue Jul 11, 2023 at 12:03 AM CDT, Michael Paquier wrote:
> >> This seems a bit incorrect because partitioning only applies to
> >> pgbench_accounts, no?  This change means that the teller and branch
> >> tables would not benefit from FREEZE under a pgbench compiled with
> >> this patch and a backend version of 14 or newer if --partitions is
> >> used.  So, perhaps "partitions" should be an argument of
> >> initPopulateTable() specified for each table?=20
> >=20
> > Whoops, looks like I didn't read the comment for what the partitions
> > variable means. It only applies to pgbench_accounts as you said. I don'=
t
> > think passing it in as an argument would make much sense. Let me know
> > what you think about the change in this new version, which only hits th=
e
> > first portion of the `if` statement if the table is pgbench_accounts.
>
> -   /* use COPY with FREEZE on v14 and later without partitioning */
> -   if (partitions =3D=3D 0 && PQserverVersion(con) >=3D 140000)
> -       copy_statement =3D "copy pgbench_accounts from stdin with (freeze=
 on)";
> +   if (partitions =3D=3D 0 && strcmp(table, "pgbench_accounts") =3D=3D 0=
 && PQserverVersion(con) >=3D 140000)
> +       copy_statement_fmt =3D "copy %s from stdin with (freeze on)";
>
> This would use the freeze option only on pgbench_accounts when no
> partitioning is defined, but my point was a bit different.  We could
> use the FREEZE option on the teller and branch tables as well, no?
> Okay, the impact is limited compared to accounts in terms of amount of
> data loaded, but perhaps some people like playing with large scaling
> factors where this could show a benefit in the initial data loading.

Perhaps, should they all be keyed off the same option? Seemed like in
your previous comment you wanted multiple options. Sorry for not reading
your comment correctly.

> In passing, I have noticed the following sentence in pgbench.sgml:
>        Using <literal>g</literal> causes logging to print one message
>        every 100,000 rows while generating data for the  =20
>        <structname>pgbench_accounts</structname> table.
> It would become incorrect as the same code path would be used for the
> teller and branch tables.

I will amend the documentation to mention all tables rather than being
specific to pgbench_accounts in the next patch revision pending what you
want to do about the partition CLI argument.

--=20
Tristan Partin
Neon (https://neon.tech)