Re: Add table access method as an option to pgbench - Mailing list pgsql-hackers

>> tablespace is an extraneous word ?

Thanks a lot for pointing this out. I will fix it in next patch once get all issues clarified.

On Sun, Dec 27, 2020 at 09:14:53AM -0400, Fabien COELHO wrote:

src/test/regress/sql/create_am.sql:CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler;
...
src/test/regress/sql/create_am.sql:DROP ACCESS METHOD heap2;

Do you suggest to add a new positive test case by using table access method *heap2* created using the existing heap_tableam_handler? 

>> If you found pre-existing inconsistencies/errors/deficiencies, I'd suggest to
>> fix them in a separate patch.  Typically those are small, and you can make them
>> 0001 patch.  Errors might be worth backpatching, so in addition to making the
>> "feature" patches small, it's good if they're separate.

>> Like writing NAME vs TABLESPACE.  Or escape vs escapes.
I will provide a separate small patch when fixing the *tablespace* typo mention above. Can you help to clarity which releases or branches need to be back patched? 

Or maybe using SET default_tablespace instead of modifying the CREATE sql.
That'd need to be done separately for indexes, and RESET after creating the
tables, to avoid accidentally affecting indexes, too.

Why should it not affect indexes?

I rarely use pgbench, and probably never looked at its source before, but I saw
that it has a separate --tablespace and --index-tablespace, and that
--tablespace is *not* the default for indexes.

So if we changed it to use SET default_tablespace instead of amending the DDL
sql, we'd need to make sure the SET applied only to the intended CREATE
command, and not all following create commands.  In the case that
--index-tablespace is not specified, it would be buggy to do this:

SET default_tablespace=foo;
CREATE TABLE ...
CREATE INDEX ...
CREATE TABLE ...
CREATE INDEX ...
...

If this `tablespace` issue also applies to `table-access-method`, yes, I agree it could be buggy too. But, the purpose of this patch is to provide an option for end user to test a newly created table access method. If the the *new* table access method hasn't fully implemented all the interfaces yet, then no matter the end user use the option provided by this patch or the GUC parameter, the results will be the same.