Re: [PATCH] Cleanup: use heap_open/heap_close consistently

On Mon, Feb 27, 2012 at 5:45 AM, Marti Raudsepp <marti@juffo.org> wrote:
> Here's a tiny cleanup: currently get_tables_to_cluster uses
> heap_open() to open the relation, but then closes it with
> relation_close(). Currently relation_close=heap_close, but it seems
> like good idea to be consistent -- in case these functions need to
> diverge in the future.

I'm inclined to fix this in the opposite way as what you've proposed:
replace heap_open() with relation_open(), rather than relation_close()
with heap_close().  The only thing heap_open() does that
relation_open() doesn't do is check the relkind, which is superfluous
here anyway; and if the check weren't superfluous it would most likely
be wrong, because heap_open rejects only some, not all, of the things
that aren't heaps.  During some of the DDL cleanup that I've been
doing during the 9.2 cycle, I've already found some cases where
careless use of heap_open rather than or in addition to an explicit
relkind check led to crappy error messages; the idea that there is
some systematic usefulness to a function that rejects indexes and
composite types (but not views, foreign tables, or sequences) doesn't
seem to be well-founded; the actual needs of people opening relations
are much more variable than that.  I'm almost inclined to think that
we should be trying to get rid of heap_open() altogether; there are
already plenty of places that assume that opening the relation is as
good as opening the heap, so I don't think there'd be any real loss of
abstraction.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company