Re: Re: [COMMITTERS] pgsql: Fix permission tests for views/tables proven empty by constraint

Greg Stark <stark(at)mit(dot)edu> writes:
> That's kind of dismaying. ORMs have a tendency to create queries like
> this and people may have even written such queries by hand and tested
> them to determine that postgres was able to exclude the useless
> relation. To have them install a security update and discover that
> something they had previously tested no longer worked would be
> annoying.

Turns out to be more to this than I realized before. In an example
such as the one I showed

select * from
((select f1 as x from t1 offset 0)
union all
(select f2 as x from t2 offset 0)) ss
where false;

where an appendrel subselect member can be proven empty on the basis
of outer-query clauses alone, *we don't even plan that subquery*.
The fix I had in mind for this fails to capture table references from
such a subquery.

We could extend setrefs.c to dig into unplanned subqueries and grab RTEs
out of them, but that would not be a complete fix. In particular, RTEs
would not get made for inheritance children of parent tables mentioned
in the query, since inheritance expansion is done by the planner. Now,
that wouldn't affect permissions checks because no extra permissions
checks are done on inheritance children, but it would affect the locking
behavior that Andres was worried about.

I think the only reliable way to make this optimization fully
transparent would be to go ahead and plan every subquery, even when we
know we'll discard the planning results immediately. That seems like
quite a lot of overkill. I'm not really sure I buy Greg's argument
that people might be depending on the performance benefits of not
planning such subqueries, but I guess it's not impossible either.

My inclination is to go ahead and write the extra code to dig RTEs out
of unplanned subqueries, and not worry about failing to lock inheritance
children in them.

I'm also now pretty firmly in the camp of "let's not try at all to fix
this in the back branches".

Thoughts?

regards, tom lane