Skip to content

Commit 6771c93

Browse files
tglsfdctglsfdc
committed
Ensure schema qualification in pg_restore DISABLE/ENABLE TRIGGER commands.
Previously, this code blindly followed the common coding pattern of passing PQserverVersion(AH->connection) as the server-version parameter of fmtQualifiedId. That works as long as we have a connection; but in pg_restore with text output, we don't. Instead we got a zero from PQserverVersion, which fmtQualifiedId interpreted as "server is too old to have schemas", and so the name went unqualified. That still accidentally managed to work in many cases, which is probably why this ancient bug went undetected for so long. It only became obvious in the wake of the changes to force dump/restore to execute with restricted search_path. In HEAD/v11, let's deal with this by ripping out fmtQualifiedId's server- version behavioral dependency, and just making it schema-qualify all the time. We no longer support pg_dump from servers old enough to need the ability to omit schema name, let alone restoring to them. (Also, the few callers outside pg_dump already didn't work with pre-schema servers.) In older branches, that's not an acceptable solution, so instead just tweak the DISABLE/ENABLE TRIGGER logic to ensure it will schema-qualify its output regardless of server version. Per bug #15338 from Oleg somebody. Back-patch to all supported branches. Discussion: https://postgr.es/m/153452458706.1316.5328079417086507743@wrigleys.postgresql.org
1 parent e4597ee commit 6771c93

File tree

7 files changed

+12
-24
lines changed

7 files changed

+12
-24
lines changed

‎src/bin/pg_dump/parallel.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1334,7 +1334,7 @@ lockTableForWorker(ArchiveHandle *AH, TocEntry *te)
13341334

13351335
query = createPQExpBuffer();
13361336

1337-
qualId = fmtQualifiedId(AH->public.remoteVersion, te->namespace, te->tag);
1337+
qualId = fmtQualifiedId(te->namespace, te->tag);
13381338

13391339
appendPQExpBuffer(query, "LOCK TABLE %s IN ACCESS SHARE MODE NOWAIT",
13401340
qualId);

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -901,9 +901,7 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
901901
ahprintf(AH, "TRUNCATE TABLE %s%s;\n\n",
902902
(PQserverVersion(AH->connection) >= 80400 ?
903903
"ONLY " : ""),
904-
fmtQualifiedId(PQserverVersion(AH->connection),
905-
te->namespace,
906-
te->tag));
904+
fmtQualifiedId(te->namespace, te->tag));
907905
}
908906

909907
/*
@@ -991,9 +989,7 @@ _disableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te)
991989
* Disable them.
992990
*/
993991
ahprintf(AH, "ALTER TABLE %s DISABLE TRIGGER ALL;\n\n",
994-
fmtQualifiedId(PQserverVersion(AH->connection),
995-
te->namespace,
996-
te->tag));
992+
fmtQualifiedId(te->namespace, te->tag));
997993
}
998994

999995
static void
@@ -1019,9 +1015,7 @@ _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te)
10191015
* Enable them.
10201016
*/
10211017
ahprintf(AH, "ALTER TABLE %s ENABLE TRIGGER ALL;\n\n",
1022-
fmtQualifiedId(PQserverVersion(AH->connection),
1023-
te->namespace,
1024-
te->tag));
1018+
fmtQualifiedId(te->namespace, te->tag));
10251019
}
10261020

10271021
/*

‎src/bin/pg_dump/pg_dump.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,9 @@ static const CatalogId nilCatalogId = {0, 0};
135135

136136
/*
137137
* Macro for producing quoted, schema-qualified name of a dumpable object.
138-
* Note implicit dependence on "fout"; we should get rid of that argument.
139138
*/
140139
#define fmtQualifiedDumpable(obj) \
141-
fmtQualifiedId(fout->remoteVersion, \
142-
(obj)->dobj.namespace->dobj.name, \
140+
fmtQualifiedId((obj)->dobj.namespace->dobj.name, \
143141
(obj)->dobj.name)
144142

145143
static void help(const char *progname);

‎src/bin/scripts/common.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,7 @@ appendQualifiedRelation(PQExpBuffer buf, const char *spec,
356356
exit(1);
357357
}
358358
appendPQExpBufferStr(buf,
359-
fmtQualifiedId(PQserverVersion(conn),
360-
PQgetvalue(res, 0, 1),
359+
fmtQualifiedId(PQgetvalue(res, 0, 1),
361360
PQgetvalue(res, 0, 0)));
362361
appendPQExpBufferStr(buf, columns);
363362
PQclear(res);

‎src/bin/scripts/vacuumdb.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,8 +406,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
406406
for (i = 0; i < ntups; i++)
407407
{
408408
appendPQExpBufferStr(&buf,
409-
fmtQualifiedId(PQserverVersion(conn),
410-
PQgetvalue(res, i, 1),
409+
fmtQualifiedId(PQgetvalue(res, i, 1),
411410
PQgetvalue(res, i, 0)));
412411

413412
simple_string_list_append(&dbtables, buf.data);

‎src/fe_utils/string_utils.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,22 +138,21 @@ fmtId(const char *rawid)
138138
}
139139

140140
/*
141-
* fmtQualifiedId - convert a qualified name to the proper format for
142-
* the source database.
141+
* fmtQualifiedId - construct a schema-qualified name, with quoting as needed.
143142
*
144143
* Like fmtId, use the result before calling again.
145144
*
146145
* Since we call fmtId and it also uses getLocalPQExpBuffer() we cannot
147146
* use that buffer until we're finished with calling fmtId().
148147
*/
149148
const char *
150-
fmtQualifiedId(int remoteVersion, const char *schema, const char *id)
149+
fmtQualifiedId(const char *schema, const char *id)
151150
{
152151
PQExpBuffer id_return;
153152
PQExpBuffer lcl_pqexp = createPQExpBuffer();
154153

155-
/* Suppress schema name if fetching from pre-7.3 DB */
156-
if (remoteVersion >= 70300 && schema && *schema)
154+
/* Some callers might fail to provide a schema name */
155+
if (schema && *schema)
157156
{
158157
appendPQExpBuffer(lcl_pqexp, "%s.", fmtId(schema));
159158
}

‎src/include/fe_utils/string_utils.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ extern PQExpBuffer (*getLocalPQExpBuffer) (void);
2525

2626
/* Functions */
2727
extern const char *fmtId(const char *identifier);
28-
extern const char *fmtQualifiedId(int remoteVersion,
29-
const char *schema, const char *id);
28+
extern const char *fmtQualifiedId(const char *schema, const char *id);
3029

3130
extern char *formatPGVersionNumber(int version_number, bool include_minor,
3231
char *buf, size_t buflen);

0 commit comments

Comments
 (0)