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.

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.

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.

--
Fabien.


Reply via email to