Re: WIP: Triggers on VIEWs

On 10 October 2010 19:06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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
>