Re: review: FDW API

On 15.02.2011 21:13, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> As the patch stands, we have to do get_rel_relkind() in a couple of
>> places in parse analysis and the planner to distinguish a foreign table
>> from a regular one. As the patch stands, there's nothing in
>> RangeTblEntry (which is what we have in transformLockingClause) or
>> RelOptInfo (for set_plain_rel_pathlist) to directly distinguish them.
>
> Hmm. I don't have a problem with adding relkind to the planner's
> RelOptInfo, but it seems to me that if parse analysis needs to know
> this, you have put functionality into parse analysis that does not
> belong there.

Possibly. We throw the existing errors, for example if you try to do
"FOR UPDATE OF foo" where foo is a set-returning function, in
transformLockingClause(), so it seemed like the logical place to check
for foreign tables too.

Hmm, one approach would be to go ahead and create the RowMarkClauses for
all relations in the parse analysis phase, foreign or not, and throw the
error later, in preprocess_rowmarks(). preprocess_rowmarks() doesn't
currently know if each RowMarkClause was created by "... FOR UPDATE" or
"FOR UPDATE OF foo", but to be consistent with the logic in
transformLockingClause() it would need to. For the former case, it would
need to just ignore foreign tables but for the latter it would need to
throw an error. I guess we could add an "explicit" flag to RowMarkClause
for that.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com