Re: review: FDW API

On Fri, Feb 18, 2011 at 10:47 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> On 15.02.2011 23:00, Tom Lane wrote:
>>> I think moving the error check downstream would be a good thing.
>
>> Ok, I tried moving the error checks to preprocess_rowmarks().
>> Unfortunately RelOptInfos haven't been built at that stage yet, so you
>> still have to do the catalog lookup to get the relkind. That defeats the
>> purpose.
>
> Mph.  It seems like the right fix here is to add relkind to
> RangeTblEntry: it could be filled in for free in addRangeTableEntry, for
> example.

Heikki and I came to the same conclusion yesterday while chatting
about this on IM.

> The main downside of that is that relation relkinds would have
> to become fixed (because there would be no practical way of updating
> RTEs in stored rules), which means the "convert relation to view" hack
> would have to go away.  Offhand I think no one cares about that anymore,
> but ...

That actually sounds like a possible problem, because it's possible to
create views with circular dependencies using CORV, and they won't
dump-and-reload properly without that hack. It's not a particularly
useful thing to do, of course, and I think we could reengineer pg_dump
to not need the hack even if someone does do it, but that sounds like
more work than we want to tackle right now.

> In any case, this is looking like a performance optimization that should
> be dealt with in a separate patch.  I'd suggest leaving the code in the
> form with the extra relkind lookups for the initial commit.  It's
> possible that no one would notice the extra lookups anyway --- have you
> benchmarked it?

This is a good point... although I think this results in at least one
extra syscache lookup per table per SELECT, even when no foreign
tables are involved.

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