pg_dump: Fix gathering of sequence information.

Since commit bd15b7db48, pg_dump uses pg_get_sequence_data() (née
pg_sequence_read_tuple()) to gather all sequence data in a single
query as opposed to a query per sequence.  Two related bugs have
been identified:

* If the user lacks appropriate privileges on the sequence, pg_dump
generates a setval() command with garbage values instead of
failing as expected.

* pg_dump can fail due to a concurrently dropped sequence, even if
the dropped sequence's data isn't part of the dump.

This commit fixes the above issues by 1) teaching
pg_get_sequence_data() to return nulls instead of erroring for a
missing sequence and 2) teaching pg_dump to fail if it tries to
dump the data of a sequence for which pg_get_sequence_data()
returned nulls.  Note that pg_dump may still fail due to a
concurrently dropped sequence, but it should now only do so when
the sequence data is part of the dump.  This matches the behavior
before commit bd15b7db48.

Bug: #19365
Reported-by: Paveł Tyślacki <pavel.tyslacki@gmail.com>
Suggested-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19365-6245240d8b926327%40postgresql.org
Discussion: https://postgr.es/m/2885944.1767029161%40sss.pgh.pa.us
Backpatch-through: 18

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 451ae6f7f69401ff80a8c60157cda8363de2cccc..a79ef0651a9e0cb4534c31be939cbd60d51afab6 100644 (file)
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1788,7 +1788,6 @@ pg_get_sequence_data(PG_FUNCTION_ARGS)
 {
 #define PG_GET_SEQUENCE_DATA_COLS      2
        Oid                     relid = PG_GETARG_OID(0);
-       SeqTable        elm;
        Relation        seqrel;
        Datum           values[PG_GET_SEQUENCE_DATA_COLS] = {0};
        bool            isnull[PG_GET_SEQUENCE_DATA_COLS] = {0};
@@ -1803,13 +1802,15 @@ pg_get_sequence_data(PG_FUNCTION_ARGS)
                                           BOOLOID, -1, 0);
        resultTupleDesc = BlessTupleDesc(resultTupleDesc);
 
-       init_sequence(relid, &elm, &seqrel);
+       seqrel = try_relation_open(relid, AccessShareLock);
 
        /*
-        * Return all NULLs for sequences for which we lack privileges, other
-        * sessions' temporary sequences, and unlogged sequences on standbys.
+        * Return all NULLs for missing sequences, sequences for which we lack
+        * privileges, other sessions' temporary sequences, and unlogged sequences
+        * on standbys.
         */
-       if (pg_class_aclcheck(relid, GetUserId(), ACL_SELECT) == ACLCHECK_OK &&
+       if (seqrel && seqrel->rd_rel->relkind == RELKIND_SEQUENCE &&
+               pg_class_aclcheck(relid, GetUserId(), ACL_SELECT) == ACLCHECK_OK &&
                !RELATION_IS_OTHER_TEMP(seqrel) &&
                (RelationIsPermanent(seqrel) || !RecoveryInProgress()))
        {
@@ -1827,7 +1828,8 @@ pg_get_sequence_data(PG_FUNCTION_ARGS)
        else
                memset(isnull, true, sizeof(isnull));
 
-       sequence_close(seqrel, NoLock);
+       if (seqrel)
+               relation_close(seqrel, AccessShareLock);
 
        resultHeapTuple = heap_form_tuple(resultTupleDesc, values, isnull);
        result = HeapTupleGetDatum(resultHeapTuple);

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9decf5a524c66da2182783b6053c855870d0ad6d..b00ba35f87ccafa9b695ccf634d8727d2d4a51fc 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -135,6 +135,7 @@ typedef struct
        int64           cache;                  /* cache size */
        int64           last_value;             /* last value of sequence */
        bool            is_called;              /* whether nextval advances before returning */
+       bool            null_seqtuple;  /* did pg_get_sequence_data return nulls? */
 } SequenceItem;
 
 typedef enum OidOptions
@@ -18785,6 +18786,7 @@ collectSequences(Archive *fout)
                sequences[i].cycled = (strcmp(PQgetvalue(res, i, 7), "t") == 0);
                sequences[i].last_value = strtoi64(PQgetvalue(res, i, 8), NULL, 10);
                sequences[i].is_called = (strcmp(PQgetvalue(res, i, 9), "t") == 0);
+               sequences[i].null_seqtuple = (PQgetisnull(res, i, 8) || PQgetisnull(res, i, 9));
        }
 
        PQclear(res);
@@ -19056,6 +19058,10 @@ dumpSequenceData(Archive *fout, const TableDataInfo *tdinfo)
        bool            called;
        PQExpBuffer query = createPQExpBuffer();
 
+       /* needn't bother if not dumping sequence data */
+       if (!fout->dopt->dumpData && !fout->dopt->sequence_data)
+               return;
+
        /*
         * For versions >= 18, the sequence information is gathered in the sorted
         * array before any calls to dumpSequenceData().  See collectSequences()
@@ -19097,6 +19103,12 @@ dumpSequenceData(Archive *fout, const TableDataInfo *tdinfo)
                entry = bsearch(&key, sequences, nsequences,
                                                sizeof(SequenceItem), SequenceItemCmp);
 
+               if (entry->null_seqtuple)
+                       pg_fatal("failed to get data for sequence \"%s\"; user may lack "
+                                        "SELECT privilege on the sequence or the sequence may "
+                                        "have been concurrently dropped",
+                                        tbinfo->dobj.name);
+
                last = entry->last_value;
                called = entry->is_called;
        }