Re: Add table access method as an option to pgbench

From: David Zhang <david(dot)zhang(at)highgo(dot)ca>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add table access method as an option to pgbench
Date: 2020-11-27 02:25:28
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Thanks a lot again for the review and comments.

On 2020-11-25 9:36 p.m., Michael Paquier wrote:
> On Wed, Nov 25, 2020 at 12:13:55PM -0800, David Zhang wrote:
>> The previous patch was based on branch "REL_13_STABLE". Now, the attached
>> new patch v2 is based on master branch. I followed the new code structure
>> using appendPQExpBuffer to append the new clause "using TABLEAM" in a proper
>> position and tested. In the meantime, I also updated the pgbench.sqml file
>> to reflect the changes.
> + <varlistentry>
> + <term><option>--table-am=<replaceable>TABLEAM</replaceable></option></term>
> + <listitem>
> + <para>
> + Create all tables with specified table access method
> + <replaceable>TABLEAM</replaceable>.
> + If unspecified, default is <literal>heap</literal>.
> + </para>
> + </listitem>
> + </varlistentry>
> This needs to be in alphabetical order.
The order issue is fixed in v3 patch.
> And what you say here is
> wrong. The default would not be heap if default_table_access_method
> is set to something else.
Right, if user change the default settings in GUC, then the default is
not `heap` any more.
> I would suggest to use table_access_method
> instead of TABLEAM,
All other options if values are required, the words are all capitalized,
such as TABLESPACE. Therefore, I used TABLEACCESSMETHOD in stead of
table_access_method in v3 patch.
> and keep the paragraph minimal, say:
> "Create tables using the specified table access method, rather than
> the default."
Updated in v3 patch.
> --table-am is really the best option name? --table-access-method
> sounds a bit more consistent to me.
Updated in v3 patch.
> + if (tableam != NULL)
> + {
> + char *escape_tableam;
> +
> + escape_tableam = PQescapeIdentifier(con, tableam, strlen(tableam));
> + appendPQExpBuffer(&query, " using %s", escape_tableam);
> + PQfreemem(escape_tableam);
> + }
Thanks for pointing this out. The order I believe is fixed in v3 patch.
> The order is wrong here, generating an unsupported grammar, see by
> yourself with this command:
> pgbench --partition-method=hash --table-am=heap -i --partitions 2

Tested in v3 patch, with a command like,

pgbench --partition-method=hash --table-access-method=heap -i --partitions 2

> This makes the patch trickier than it looks as the USING clause is
> located between PARTITION BY and WITH. Also, partitioned tables
> cannot use the USING clause so you need to be careful that
> createPartitions() also uses the correct table AM.
> This stuff needs tests.
3 test cases has been added the tap test, but honestly I am not sure if
I am following the rules. Any comments will be great.
> --
> Michael

Software Engineer
Highgo Software Inc. (Canada)

Attachment Content-Type Size
v3-0001-add-table-access-method-as-an-option-to-pgbench.patch text/plain 5.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message David Zhang 2020-11-27 02:33:44 Re: Add table access method as an option to pgbench
Previous Message 2020-11-27 02:19:57 RE: [Patch] Optimize dropping of relation buffers using dlist