On 23 January 2014 06:13, KaiGai Kohei <kaigai@ak.jp.nec.com> wrote:
> A minor comment is below:
>
> ! /*
> ! * Make sure that the query is marked correctly if the added
> qual
> ! * has sublinks.
> ! */
> ! if (!parsetree->hasSubLinks)
> ! parsetree->hasSubLinks = checkExprHasSubLink(viewqual);
>
> Is this logic really needed? This flag says the top-level query
> contains SubLinks, however, the above condition checks whether
> a sub-query to be constructed shall contain SubLinks.
> Also, securityQuals is not attached to the parse tree right now.
>
Thanks for looking at this.
Yes that logic is needed. Without it the top-level query wouldn't be
marked as having sublinks, which could cause all sorts of things to go
wrong --- for example, without it fireRIRrules() wouldn't walk the
query tree looking for SELECT rules in sublinks to expand, so a
security barrier qual with a sublink subquery that selected from
another view wouldn't work.
It is not true to say that "securityQuals is not attached to the parse
tree". The securityQuals *are* part of the parse tree, they just
happen to be held in a different place to keep them isolated from
other quals. So the remaining rewriter code that walks or mutates the
parse tree will process them just like any other quals, recursively
expanding any rules they contain.
Regards,
Dean