Commit 3eb9a5e7 authored by Tom Lane's avatar Tom Lane

Fix pg_dump/pg_restore to emit REFRESH MATERIALIZED VIEW commands last.

Because we push all ACL (i.e. GRANT/REVOKE) restore steps to the end,
materialized view refreshes were occurring while the permissions on
referenced objects were still at defaults.  This led to failures if,
say, an MV owned by user A reads from a table owned by user B, even
if B had granted the necessary privileges to A.  We've had multiple
complaints about that type of restore failure, most recently from
Jordan Gigov.

The ideal fix for this would be to start treating ACLs as dependency-
sortable objects, rather than hard-wiring anything about their dump order
(the existing approach is a messy kluge dating to commit dc0e76ca).
But that's going to be a rather major change, and it certainly wouldn't
lead to a back-patchable fix.  As a short-term solution, convert the
existing two-pass hack (ie, normal objects then ACLs) to a three-pass hack,
ie, normal objects then ACLs then matview refreshes.  Because this happens
in RestoreArchive(), it will also fix the problem when restoring from an
existing archive-format dump.

(Note this means that if a matview refresh would have failed under the
permissions prevailing at dump time, it'll fail during restore as well.
We'll define that as user error rather than something we should try
to work around.)

To avoid performance loss in parallel restore, we need the matview
refreshes to still be parallelizable.  Hence, clean things up enough
so that both ACLs and matviews are handled by the parallel restore
infrastructure, instead of reverting back to serial restore for ACLs.
There is still a final serial step, but it shouldn't normally have to
do anything; it's only there to try to recover if we get stuck due to
some problem like unresolved circular dependencies.

Patch by me, but it owes something to an earlier attempt by Kevin Grittner.
Back-patch to 9.3 where materialized views were introduced.

Discussion: https://postgr.es/m/28572.1500912583@sss.pgh.pa.us
parent 9a3b5d3a
This diff is collapsed.
......@@ -203,6 +203,30 @@ typedef enum
OUTPUT_OTHERDATA /* writing data as INSERT commands */
} ArchiverOutput;
/*
* For historical reasons, ACL items are interspersed with everything else in
* a dump file's TOC; typically they're right after the object they're for.
* However, we need to restore data before ACLs, as otherwise a read-only
* table (ie one where the owner has revoked her own INSERT privilege) causes
* data restore failures. On the other hand, matview REFRESH commands should
* come out after ACLs, as otherwise non-superuser-owned matviews might not
* be able to execute. (If the permissions at the time of dumping would not
* allow a REFRESH, too bad; we won't fix that for you.) These considerations
* force us to make three passes over the TOC, restoring the appropriate
* subset of items in each pass. We assume that the dependency sort resulted
* in an appropriate ordering of items within each subset.
* XXX This mechanism should be superseded by tracking dependencies on ACLs
* properly; but we'll still need it for old dump files even after that.
*/
typedef enum
{
RESTORE_PASS_MAIN = 0, /* Main pass (most TOC item types) */
RESTORE_PASS_ACL, /* ACL item types */
RESTORE_PASS_REFRESH /* Matview REFRESH items */
#define RESTORE_PASS_LAST RESTORE_PASS_REFRESH
} RestorePass;
typedef enum
{
REQ_SCHEMA = 0x01, /* want schema */
......@@ -329,6 +353,7 @@ struct _archiveHandle
int noTocComments;
ArchiverStage stage;
ArchiverStage lastErrorStage;
RestorePass restorePass; /* used only during parallel restore */
struct _tocEntry *currentTE;
struct _tocEntry *lastErrorTE;
};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment