Thanks a lot for the comments, Fabien.
> Some feedback about v3:
>
> In the doc I find TABLEACCESSMETHOD quite hard to read. Use TABLEAM
> instead?
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
*using* clause.
>
> About tests :
>
> They should also trigger failures, eg
> "--table-access-method=no-such-table-am", to be added to the @errors
> list.
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.
>
--
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca
