On Tue, Sep 13, 2011 at 4:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Sep 10, 2011, at 11:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> (IOW, rather than "fix" this I'd prefer to rip out the code altogether.
>>> But maybe we should wait a couple more years for that.)
>
>> IIRC, it's not dead code. I think you can still generate such a dump if you use CREATE OR REPLACE VIEW to
manufacturea pair of mutually recursive views.
>
> Oh, yeah, I'd forgotten about that. In general that's pg_dump's
> strategy for breaking a circular dependency loop that involves a view.
>
>> Now we should probably disallow that, but we currently don't.
>
> Losing that particular case isn't problematic, but I'm not sure that
> that's the only possible circularity involving a view. One idea that
> comes to mind is
>
> create table foo (list-of-columns);
>
> create function foofunc () returns setof foo as ...;
>
> create rule .... as select * from foofunc();
>
> This only saves somebody from citing the list of column types twice,
> so maybe we could blow off this case too; but who's to say there are
> not more-useful cases that would create circularities?
I spent some more time looking at this tonight. I am wondering if
perhaps we should just get rid of relhasindex. At the time that was
added - which predates our commit history - we didn't use the relcache
to cache the results of scanning pg_index, and we scanned pg_index
using a heap scan rather than an index scan. So at that point being
able to tell from the pg_class tuple that no indexes could exist was
probably a rather large win. It's likely a lot smaller now - and,
really, how many tables don't have indexes anyway? Even so, I'm not
100% sure whether it's worth doing: the existing code isn't really
hurting us, and I think we could fix Thom's complaint by changing
DefineQueryRewrite() to call RelationGetIndexList() rather than
blindly believing relhasindex, which would be maybe a five line
code-change. We'd probably also want to change
SetRelationRuleStatus() to clear relhasindex, which would be one more
line of code.
One related thing that seems worth doing is ripping out relhaspkey,
which is used for absolutely nothing at all. It looks to me like
doing so will save four bytes per row in pg_class due to alignment
padding. Patch for that attached. (Yeah, I know... this could break
client code... but is it really worth keeping this cruft around
forever? Especially since the column can easily say there's a primary
key when there really isn't?)
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
