|From:||David Zhang <david(dot)zhang(at)highgo(dot)ca>|
|To:||Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>|
|Cc:||Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>|
|Subject:||Re: Add table access method as an option to pgbench|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Thanks a lot for the comments, Fabien.
> Some feedback about v3:
> In the doc I find TABLEACCESSMETHOD quite hard to read. Use TABLEAM
Yes, in this case, *TABLEAM* is easy to read, updated.
> Also, the next entry uses lowercase (tablespace), why change the style?
The original style is not so consistent, for example, in doc,
--partition-method using *NAME*, but --table-space using *tablespace*;
in help, --partition-method using *(rang|hash)*, but --tablespace using
*TABLESPACE*. To keep it more consistent, I would rather use *TABLEAM*
in both doc and help.
> Remove space after comma in help string. I'd use the more readable
> TABLEAM in the help string rather than the mouthful.
Yes the *space* is removed after comma.
> It looks that the option is *silently* ignored when creating a
> partitionned table, which currently results in an error, and not
> passed to partitions, which would accept them. This is pretty weird.
The input check is added with an error message when both partitions and
table access method are used.
> I'd suggest that the table am option should rather by changing the
> default instead, so that it would apply to everything relevant
> implicitely when appropriate.
I think this is a better idea, so in v4 patch I change it to something
like "set default_table_access_method to *TABLEAM*" instead of using the
> About tests :
> They should also trigger failures, eg
> "--table-access-method=no-such-table-am", to be added to the @errors
No sure how to address this properly, since the table access method
potentially can be *any* name.
> I do not understand why you duplicated all possible option entry.
> Test with just table access method looks redundant if the feature is
> make to work orthonogonally to partitions, which should be the case.
Only one positive test case added using *heap* as the table access
method to verify the initialization.
>> By the way, I saw the same style for other variables, such as
>> escape_tablespace, should this be fixed as well?
> Nope, let it be.
Highgo Software Inc. (Canada)
|Next Message||Fujii Masao||2020-12-02 04:52:43||Re: Add statistics to pg_stat_wal view for wal related parameter tuning|
|Previous Message||Julien Rouhaud||2020-12-02 04:05:16||pg_stat_statements oddity with track = all|