Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> Hmm. I've not read any of the new code yet, but the fact that this
> test now reduces to a one-time filter makes it effectively useless as
> a test of qual evaluation order because it has deduced that it doesn't
> need to evaluate them. I would suggest replacing the qual with
> something that can't be reduced, perhaps "2*a = 6".
That's a good thought, I agree.
> In addition, I think that the tests on this view are probably no
> longer adequate for the purpose of validating that the qual evaluation
> order is safe. With the old implementation, the subquery scans in the
> plans made it pretty clear that it was safe, and likely to remain safe
> with variants of those queries, but that's not so obvious with the new
> plans. Maybe some additional quals could be added to the view
> definition, perhaps based on the other view columns, to verify that
> the outer leaky qual always gets evaluated after the security barrier
> quals, regardless of cost. Or perhaps that's something that's better
> proved with an all-new set of tests, but it does seem to me that the
> new implementation has a higher risk (or at least introduces different
> risks) of unsafe evaluation orders that warrant some additional
> testing.
This also sounds like a good idea to me. I'm not sure how practical it
would be in this case, but I do think it might be a good idea to also
review the code coverage results and see if there are tests which could
improve wherever it is lacking.
Thanks!
Stephen
