On 10 October 2010 19:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Applied with revisions.
Brilliant! Thank you very much.
> * 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?
No, I think you're right. It was just the leftovers of an early
attempt to get the rewriter changes right.
> * 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.
Ah yes, I missed that. I still don't see where it uses those TLEs by
name though. It looks as though it's using wholeAttNo, so perhaps my
code would have worked if I had set wholeAttNo on the RowMark? Anyway,
I don't think it's likely that this extra Var is going to be present
often in practice, so I don't think it's a problem worth worrying
about.
Thanks very much for looking at this.
Regards,
Dean
> 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
>