Re: Add table access method as an option to pgbench

Again, thanks a lot for the feedback.

On 2020-12-02 12:03 p.m., Fabien COELHO wrote:
>
> Hello David,
>
> Some feedback about v4.
>
>>> 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.
>
> Hmmm. If you take the resetting the default, I do not think that you 
> should have to test anything? AFAICT the access method is valid on 
> partitions, although not on the partitioned table declaration. So I'd 
> say that you could remove the check.
Yes, it makes sense to remove the *blocking* check, and actually the 
table access method interface does work with partitioned table. I tested 
this/v5 by duplicating the heap access method with a different name. For 
this reason, I removed the condition check as well when applying the 
table access method.
>
>>> 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 think it is pretty unlikely that someone would chose the name 
> "no-such-table-am" when developing a new storage engine for Postgres 
> inside Postgres, so that it would make this internal test fail.
>
> There are numerous such names used in tests, eg "no-such-database", 
> "no-such-command".
>
> So I'd suggest to add such a test to check for the expected failure.
The test case "no-such-table-am" has been added to the errors list, and 
the regression test is ok.
>
>>> 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.
>
> Yep, only "heap" if currently available for tables.
>
Thanks,
-- 
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca