Re: BUG #13907: Restore materialized view throw permission denied - Mailing list pgsql-bugs
From | Kevin Grittner |
---|---|
Subject | Re: BUG #13907: Restore materialized view throw permission denied |
Date | |
Msg-id | CACjxUsNzm_PwMfHdJkXX9UVOm3hkSvs5fJ8LzkqCHxdBXHDdEQ@mail.gmail.com Whole thread Raw |
In response to | Re: BUG #13907: Restore materialized view throw permission denied (Tom Lane <tgl@sss.pgh.pa.us>) |
List | pgsql-bugs |
On Tue, Jul 26, 2016 at 1:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Kevin Grittner <kgrittn@gmail.com> writes: >> On Tue, Jul 26, 2016 at 12:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> It's possible that the most reasonable fix is to move matview refreshes to >>> after the ACL restoration step. That would wreak a lot of havoc with >>> the current view of a dump as being tripartite (pre-data/data/post-data), >>> so it might be more work than we want to do. But it seems like the >>> logically soundest thing. > >> That is exactly what the patch I posted yesterday does. Peter was >> suggesting that wasn't good enough > > Well, he's right: just minimally rearranging the dump order is not enough. > For one thing, I'm pretty sure this patch will fail to fix the bug in a > parallel restore, because restore_toc_entries_parallel has hard-wired > knowledge that ACLs can be done last and serially. I think I covered that. I tested all dump formats, including parallel; but it is possible there was something wrong with my testing. > Even if the fix works > in parallel restore, it would result in doing matview refreshes serially > not in parallel, which is a pretty tough nut to swallow. ok > And I'm not > really convinced that the patch preserves dependency ordering requirements > at all, even in plain serial restore. Pre-patch, it passes the sorted objects twice -- once for everything except ACLs and again for ACLs. I just made sure that refreshes sorted past ACLs and moved them to the ACL phase. I don't see why that would disturb the order in which the objects are passed either time; just what the filtering is on each pass. > The core of the problem here, I think, is that pg_dump has never had any > real notion of dependency ordering for ACLs, since it's always been > sufficient to dump them at the very end in arbitrary order. If we're now > going to make objects-with-dependencies also dependent on ACLs, I'm afraid > that raises the bar quite a lot in terms of needing to treat ACLs as > real, dependency-sorted objects. I was wondering why we went outside the normal ordering logic for ACLs -- seems like a pretty ugly hack, but I didn't figure it was the job of this patch to fix that. > I believe the thrust of Peter's suggestion is to try to avoid biting that > bullet, by instead instituting a rule of "dump an object's ACL immediately > after the object". But to make this work with the read-only-table > scenario, it would have to be something like "dump an object's GRANTs > immediately after the object, but if there are any self-revokes, dump > those last as we always have". I see what you're saying, but read-only-tables aren't the problem. The only problem this patch leaves, I think, is that a matview for which objects it depends on have changed since the last REFRESH (or CREATE WITH DATA) might not be able to refresh at the end; but I don't see how you avoid that with policies, functions, or views -- so I'm not clear why we care more about ACLs in that regard. > With either approach, I'm afraid we're talking about a patch much larger > and more invasive than what's here :-( Well, this allows the example from the bug report to run successfully. It seems to me to generate dumps that are clearly better that what are generated without the patch; so perhaps this is worthwhile for now (with more appropriate tests, of course) while we sort out the perfect solution for somewhere down the line. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
pgsql-bugs by date: