From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Marti Raudsepp <marti(at)juffo(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] Cleanup: use heap_open/heap_close consistently |
Date: | 2012-02-27 17:35:28 |
Message-ID: | CA+TgmoanRXV6Fr-gk-RyrGFEBx3vqyZJvxPg9=5=gd6CKXHGxA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Feb 27, 2012 at 11:59 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> 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.
>
> Or, perhaps, restrict it to open actual heaps (ie, relkind 'r')?
That carries a significant risk of breaking third-party code; or even
core code. I'm almost positive that there is core code that relies on
heap_open's failure to reject all relkinds other than 'r'. We can go
through all the callers and audit them, but there's a non-trivial risk
of breaking something.
> I think that if you count, you'll find the vast majority of heap_open
> calls are really opening system catalogs. So I'd just as soon have
> a relkind check there for sanity's sake, not to mention that renaming
> them all creates a lot of unnecessary code churn.
The code churn does suck. I have to admit, though, that I'd really
like to get out from under the pairing requirement: we've pretty much
already committed ourselves to a future where heap_close() can never
be anything more than relation_close(). Like it or not, that die is
cast. In doing the DDL refactoring that I undertook this release
cycle, I found that it was often necessary to switch from using
heap_openrv() to RangeVarGetRelid + relation_open(). That of course
means tracking down the corresponding heap_close() calls and making
them relation_close(). I think it might be better to bite the bullet
and just do that across the board. In the long run I think life will
be simpler with just one way to do it (Perl slogans nonwithstanding).
> IMO it would be sensible for heap_open to insist on a heap, index_open
> to insist on an index, and for anything else, use relation_open and
> BYO relkind check. There are a few common patterns (eg "does relation
> have storage") that we should abstract somehow, but it might be better
> to provide separate relkind-check routines than to invent xxx_open.
Definitely.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Pavan Deolasee | 2012-02-27 17:43:39 | Re: How to know a table has been modified? |
Previous Message | Tom Lane | 2012-02-27 17:34:31 | Re: pgstat documentation tables |