On Thu, Feb 9, 2017 at 7:21 AM, Corey Huinker <corey.huin...@gmail.com> wrote: > Is this still needing a reviewer?
Useful input is always welcome. > Code is quite clear. It does raise two questions: > > 1. should have these tests named in core functions, like maybe: > > relation_has_visibility(Relation) > relation_has_storage(Relation) > and/or corresponding void functions check_relation_has_visibility() and > check_relation_has_storage() which would raise the appropriate error message > when the boolean test fails. > Then again, this might be overkill since we don't add new relkinds very > often. The visibility checks are localized in pg_visibility.c and the storage checks in pgstatindex.c, so yes we could have macros in those files. Or even better: just have a sanity check routine as the error messages are the same everywhere. > 2. Is it better stylistically to have an AND-ed list of != tests, or a > negated list of OR-ed equality tests, and should the one style be changed to > conform to the other? > > No new regression tests. I think we should create a dummy partitioned table > for each contrib and show that it fails. Yep, good point. That's easy enough to add. By the way, partition tables create a file on disk but they should not... Amit, I think you are working on a patch for that as well? That's tweaking heap_create() to unlist partitioned tables and then be sure that other code paths don't try to look at the parent partitioned table on disk. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
