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 |
Date: | 2020-12-07 21:53:33 |
Message-ID: | 395a9af4-b514-38de-ece9-283968b8cc6c@highgo.ca |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v5-0001-add-table-access-method-as-an-option-to-pgbench.patch | text/plain | 4.7 KB |
From | Date | Subject | |
---|---|---|---|
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 |