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. And what you say here is
wrong. The default would not be heap if default_table_access_method
is set to something else. I would suggest to use table_access_method
instead of TABLEAM, and keep the paragraph minimal, say:
"Create tables using the specified table access method, rather than
the default."
--table-am is really the best option name? --table-access-method
sounds a bit more consistent to me.
+ if (tableam != NULL)
+ {
+ char *escape_tableam;
+
+ escape_tableam = PQescapeIdentifier(con, tableam, strlen(tableam));
+ appendPQExpBuffer(&query, " using %s", escape_tableam);
+ PQfreemem(escape_tableam);
+ }
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
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.
--
Michael
