Обсуждение: uncataloged tables are a vestigial husk

While working on some code today, I noticed that RELKIND_UNCATALOGED
appears to serve no useful purpose.  In the few places where we check
for it at all, we treat it in exactly the same way as
RELKIND_RELATION.  It seems that it's only purpose is to serve as a
placeholder inside each newly-created relcache entry until the real
relkind is filled in.  But this seems pretty silly, because
RelationBuildLocalRelation(), where the relcache entry is created, is
called in only one place, heap_create(), which already knows the
relkind.  So, essentially, right now, we're relying on the callers of
heap_create() to pass in a relkind and then, after heap_create()
returns, stick that same relkind into the relcache entry before
inserting the pg_class tuple.  The only place where that doesn't
happen is in the bootstrap code, which instead allows
RELKIND_UNCATALOGED to stick around in the relcache entry even though
we have RELKIND_RELATION in the pg_class tuple. But we don't actually
rely on that for anything, so it seems this is just an unnecessary
complication.

The attached patch cleans it up by removing RELKIND_UNCATALOGED and
teaching RelationBuildLocalRelation() to set the relkind itself.

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

Robert Haas <robertmhaas@gmail.com> writes:
> While working on some code today, I noticed that RELKIND_UNCATALOGED
> appears to serve no useful purpose.  In the few places where we check
> for it at all, we treat it in exactly the same way as
> RELKIND_RELATION.  It seems that it's only purpose is to serve as a
> placeholder inside each newly-created relcache entry until the real
> relkind is filled in.

I suspect that it had some actual usefulness back in Berkeley days.
But now that catalogs are created with the correct relkind to start
with during initdb, I agree it's probably just inertia keeping that
around.

> The attached patch cleans it up by removing RELKIND_UNCATALOGED and
> teaching RelationBuildLocalRelation() to set the relkind itself.

I think there are probably some places to fix in the docs too.
        regards, tom lane

On Wed, Jun 13, 2012 at 1:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The attached patch cleans it up by removing RELKIND_UNCATALOGED and
>> teaching RelationBuildLocalRelation() to set the relkind itself.
>
> I think there are probably some places to fix in the docs too.

catalogs.sgml doesn't include it in the list of possible relkinds,
since it never hits the disk.  And grep -i uncatalog doc/src/sgml
comes up empty.  Where else should I be looking?

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jun 13, 2012 at 1:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The attached patch cleans it up by removing RELKIND_UNCATALOGED and
>>> teaching RelationBuildLocalRelation() to set the relkind itself.

>> I think there are probably some places to fix in the docs too.

> catalogs.sgml doesn't include it in the list of possible relkinds,
> since it never hits the disk.  And grep -i uncatalog doc/src/sgml
> comes up empty.  Where else should I be looking?

Huh.  Okay, there probably isn't anyplace then.  I'm surprised we didn't
list it in catalogs.sgml, though.
        regards, tom lane