From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Greg Stark <stark(at)mit(dot)edu>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Re: [COMMITTERS] pgsql: Fix permission tests for views/tables proven empty by constraint |
Date: | 2013-05-08 18:00:02 |
Message-ID: | 20130508180002.GE7901@awork2.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On 2013-05-08 12:30:31 -0400, Tom Lane wrote:
> 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 first thought that is fair enough since I thought that in most if not
all cases where locking plays a user visible role the parent relation
would get locked anyway when a child relations gets locked. Turns out,
we do it only the other way round, i.e. lock child relations when we
lock a parent relation, even for most ddl in child relations.
I am not sure if its really problematic, but it seems to allow scenarios
like:
S1: BEGIN;
S1: SELECT * FROM ((SELECT * FROM parent OFFSET 0) UNION ALL (SELECT * FROM parent OFFSET 0)) f WHERE false;
-- parent is locked now, children are not
S2: BEGIN;
S2: ALTER TABLE child_1 DROP CONSTRAINT foo;
S1: SELECT * FROM parent WHERE ...
-- blocks, waiting for S1 since child_1 is locked.
This seems like somewhat confusing behaviour, although it has gone
unnoticed so far, since one normally expect that a previous lock allows
you to continue workin with a relation.
But I guess this is better fixed by making all DDL on child relations
also lock their parent relation? That seems like a good idea anyway.
I am not at all convinced that this must be fixed, but also not the
other way round. I just wanted to point this out since I am not sure
there aren't any more problematic cases.
> 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.
I didn't understand Greg's argument as being based on performance but as
being worried about the changed locking and such from a functional
perspective. Greg?
I don't really buy the performance argument either, but I agree that we
shouldn't do all this in the back branches as the "bug" isn't really bad
and it has some potential for introducing problems.
> I'm also now pretty firmly in the camp of "let's not try at all to fix
> this in the back branches".
+1 independent of where this goes.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2013-05-08 18:00:44 | pgsql: The data structure used in unaccent is a trie, not suffix tree. |
Previous Message | Heikki Linnakangas | 2013-05-08 17:31:00 | pgsql: Fix walsender failure at promotion. |
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2013-05-08 18:01:04 | Re: Terminology issue: suffix tree |
Previous Message | David Fetter | 2013-05-08 17:55:40 | Re: RETURNING syntax for COPY |