On 28 November 2016 at 23:55, Stephen Frost <sfr...@snowman.net> wrote: >> diff --git a/src/test/regress/expected/updatable_views.out >> b/src/test/regress/expected/updatable_views.out > [...] >> --- 2104,2114 ---- >> >> EXPLAIN (VERBOSE, COSTS OFF) >> UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a = 3; >> ! QUERY PLAN >> ! -------------------------- >> ! Result >> ! One-Time Filter: false >> ! (2 rows) > > Perhaps Dean recalls something more specific, but reviewing this test > and the others around it, I believe it was simply to see what happens in > a case which doesn't pass the security barrier view constraint and to > cover the same cases with the UPDATE as were in the SELECTs above it. I > don't see it being an issue that it now results in a one-time filter: > false result. >
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". 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. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers