|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|
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
>> 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",
> 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.
Highgo Software Inc. (Canada)
|Next Message||Tom Lane||2020-12-07 21:58:28||Re: Commitfest statistics|
|Previous Message||Tom Lane||2020-12-07 21:32:32||Re: [HACKERS] [PATCH] Generic type subscripting|