Re: 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(at)gmail(dot)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(at)sss(dot)pgh(dot)pa(dot)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(at)gmail(dot)com> writes:
> On Wed, Jun 13, 2012 at 1:13 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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