Bernd Helmle <mailings@oopsware.de> writes:
> --On 8. September 2010 09:00:33 +0100 Dean Rasheed
> <dean.a.rasheed@gmail.com> wrote:
>> Here's an updated version with improved formatting and a few minor
>> wording changes to the triggers chapter.
> This version doesn't apply clean anymore due to some rejects in
> plainmain.c. Corrected version attached.
Applied with revisions. A couple of points worth remarking:
* I took out this change in planmain.c:
+ /*
+ * If the query target is a VIEW, it won't be in the jointree, but we
+ * need a dummy RelOptInfo node for it. This need not have any stats in
+ * it because it always just goes at the top of the plan tree.
+ */
+ if (parse->resultRelation &&
+ root->simple_rel_array[parse->resultRelation] == NULL)
+ build_simple_rel(root, parse->resultRelation, RELOPT_OTHER_MEMBER_REL);
AFAICT that's just dead code: the only reason to build such a rel would
be if there were Vars referencing it in the main part of the plan tree.
But there aren't. Perhaps this was left over from some early iteration
of the patch before you had the Var numbering done right? Do you know
of any cases where it's still needed?
* I also took out the changes in preprocess_targetlist() that tried to
prevent equivalent wholerow vars from getting entered in the targetlist.
This would not work as intended since the executor has specific
expectations for the names of those junk TLEs; it'd fail if it ever
actually tried to do an EvalPlanQual recheck that needed those TLEs.
Now I believe that an EPQ recheck is impossible so far as the update or
delete itself is concerned, when the target is a view. So if you were
really concerned about the extra vars, the non-kluge route to a solution
would be to avoid generating RowMarks in the first place. You'd have to
think a bit about the possibility of SELECT FOR UPDATE in sub-selects
though; the query as a whole might need some rowmarks even if the top
level Modify node doesn't. On the whole I couldn't get excited about
this issue, so I just left it alone.
regards, tom lane