Re: review: FDW API

I wrote:
> I did a bit of poking around here, and came to the following
> conclusions:

> 1. We don't want to add another RTEKind. RTE_RELATION basically has the
> semantics of "anything with a pg_class OID", so it ought to include
> foreign tables. Therefore the fix ought to be to add relkind to
> RangeTblEntry. (BTW, so far as I can tell, RTE_SPECIAL is obsolete:
> there are assorted switch cases that handle it, but no place that can
> generate the value. I'm inclined to delete it while we are messing
> with this.)

> 2. In the current code layout, to make sense of relkind you need to
> #include catalog/pg_class.h where the values for relkind are #defined.
> I dislike the idea of that being true for a field of such a widely-known
> struct as RangeTblEntry. Accordingly, I suggest that we move those
> values into parsenodes.h. (Perhaps we could convert them to an enum,
> too, though still keeping the same ASCII values.)

> 3. We can have the rewriter update an RTE's stored value of relkind
> during AcquireRewriteLocks: it opens the rel for each RTE_RELATION entry
> anyway, so copying over the relkind is essentially free. While it's not
> instantly obvious that that is "soon enough", I think that it is, since
> up to the point of acquiring a lock there we can't assume that the rel
> isn't being changed or dropped undeneath us --- that is, any earlier
> test on an RTE's relkind might be testing just-obsoleted state anyway.

> 4. I had hoped that we might be able to get rid of some pre-existing
> syscache lookups, but at least so far as the parse/plan/execute chain
> is concerned, there don't seem to be any other places that need to
> fetch a relkind based on just an RTE entry.

> So point #4 is a bit discouraging, but other that, this seems like
> a fairly straightforward exercise. I'm inclined to go ahead and do it,
> unless there are objections.

Applied, except I ended up not moving the RELKIND #defines as suggested
in point #2. Those #defines are used by pg_dump, and having pg_dump.c
#include parsenodes.h seemed like a bad idea.

regards, tom lane