Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit d645273

Browse files
tglsfdctglsfdc
committed
Rethink function argument sorting in pg_dump.
Commit 7b583b2 created an unnecessary dump failure hazard by applying pg_get_function_identity_arguments() to every function in the database, even those that won't get dumped. This could result in snapshot-related problems if concurrent sessions are, for example, creating and dropping temporary functions, as noted by Marko Tiikkaja in bug #12832. While this is by no means pg_dump's only such issue with concurrent DDL, it's unfortunate that we added a new failure mode for cases that used to work, and even more so that the failure was created for basically cosmetic reasons (ie, to sort overloaded functions more deterministically). To fix, revert that patch and instead sort function arguments using information that pg_dump has available anyway, namely the names of the argument types. This will produce a slightly different sort ordering for overloaded functions than the previous coding; but applying strcmp directly to the output of pg_get_function_identity_arguments really was a bit odd anyway. The sorting will still be name-based and hence independent of possibly-installation-specific OID assignments. A small additional benefit is that sorting now works regardless of server version. Back-patch to 9.3, where the previous commit appeared.
1 parent 49bb343 commit d645273

File tree

3 files changed

+22
-49
lines changed

3 files changed

+22
-49
lines changed

‎src/bin/pg_dump/pg_dump.c

Lines changed: 2 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3912,7 +3912,6 @@ getAggregates(Archive *fout, int *numAggs)
39123912
int i_proargtypes;
39133913
int i_rolname;
39143914
int i_aggacl;
3915-
int i_proiargs;
39163915

39173916
/* Make sure we are in proper schema */
39183917
selectSourceSchema(fout, "pg_catalog");
@@ -3922,12 +3921,11 @@ getAggregates(Archive *fout, int *numAggs)
39223921
* rationale behind the filtering logic.
39233922
*/
39243923

3925-
if (fout->remoteVersion >= 80400)
3924+
if (fout->remoteVersion >= 80200)
39263925
{
39273926
appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
39283927
"pronamespace AS aggnamespace, "
39293928
"pronargs, proargtypes, "
3930-
"pg_catalog.pg_get_function_identity_arguments(oid) AS proiargs,"
39313929
"(%s proowner) AS rolname, "
39323930
"proacl AS aggacl "
39333931
"FROM pg_proc p "
@@ -3945,28 +3943,12 @@ getAggregates(Archive *fout, int *numAggs)
39453943
"deptype = 'e')");
39463944
appendPQExpBuffer(query, ")");
39473945
}
3948-
else if (fout->remoteVersion >= 80200)
3949-
{
3950-
appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
3951-
"pronamespace AS aggnamespace, "
3952-
"pronargs, proargtypes, "
3953-
"NULL::text AS proiargs,"
3954-
"(%s proowner) AS rolname, "
3955-
"proacl AS aggacl "
3956-
"FROM pg_proc p "
3957-
"WHERE proisagg AND ("
3958-
"pronamespace != "
3959-
"(SELECT oid FROM pg_namespace "
3960-
"WHERE nspname = 'pg_catalog'))",
3961-
username_subquery);
3962-
}
39633946
else if (fout->remoteVersion >= 70300)
39643947
{
39653948
appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
39663949
"pronamespace AS aggnamespace, "
39673950
"CASE WHEN proargtypes[0] = 'pg_catalog.\"any\"'::pg_catalog.regtype THEN 0 ELSE 1 END AS pronargs, "
39683951
"proargtypes, "
3969-
"NULL::text AS proiargs, "
39703952
"(%s proowner) AS rolname, "
39713953
"proacl AS aggacl "
39723954
"FROM pg_proc "
@@ -3981,7 +3963,6 @@ getAggregates(Archive *fout, int *numAggs)
39813963
"0::oid AS aggnamespace, "
39823964
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
39833965
"aggbasetype AS proargtypes, "
3984-
"NULL::text AS proiargs, "
39853966
"(%s aggowner) AS rolname, "
39863967
"'{=X}' AS aggacl "
39873968
"FROM pg_aggregate "
@@ -3997,7 +3978,6 @@ getAggregates(Archive *fout, int *numAggs)
39973978
"0::oid AS aggnamespace, "
39983979
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
39993980
"aggbasetype AS proargtypes, "
4000-
"NULL::text AS proiargs, "
40013981
"(%s aggowner) AS rolname, "
40023982
"'{=X}' AS aggacl "
40033983
"FROM pg_aggregate "
@@ -4021,7 +4001,6 @@ getAggregates(Archive *fout, int *numAggs)
40214001
i_proargtypes = PQfnumber(res, "proargtypes");
40224002
i_rolname = PQfnumber(res, "rolname");
40234003
i_aggacl = PQfnumber(res, "aggacl");
4024-
i_proiargs = PQfnumber(res, "proiargs");
40254004

40264005
for (i = 0; i < ntups; i++)
40274006
{
@@ -4041,7 +4020,6 @@ getAggregates(Archive *fout, int *numAggs)
40414020
agginfo[i].aggfn.lang = InvalidOid; /* not currently interesting */
40424021
agginfo[i].aggfn.prorettype = InvalidOid; /* not saved */
40434022
agginfo[i].aggfn.proacl = pg_strdup(PQgetvalue(res, i, i_aggacl));
4044-
agginfo[i].aggfn.proiargs = pg_strdup(PQgetvalue(res, i, i_proiargs));
40454023
agginfo[i].aggfn.nargs = atoi(PQgetvalue(res, i, i_pronargs));
40464024
if (agginfo[i].aggfn.nargs == 0)
40474025
agginfo[i].aggfn.argtypes = NULL;
@@ -4093,7 +4071,6 @@ getFuncs(Archive *fout, int *numFuncs)
40934071
int i_proargtypes;
40944072
int i_prorettype;
40954073
int i_proacl;
4096-
int i_proiargs;
40974074

40984075
/* Make sure we are in proper schema */
40994076
selectSourceSchema(fout, "pg_catalog");
@@ -4114,13 +4091,12 @@ getFuncs(Archive *fout, int *numFuncs)
41144091
* doesn't have; otherwise we might not get creation ordering correct.
41154092
*/
41164093

4117-
if (fout->remoteVersion >= 80400)
4094+
if (fout->remoteVersion >= 70300)
41184095
{
41194096
appendPQExpBuffer(query,
41204097
"SELECT tableoid, oid, proname, prolang, "
41214098
"pronargs, proargtypes, prorettype, proacl, "
41224099
"pronamespace, "
4123-
"pg_catalog.pg_get_function_identity_arguments(oid) AS proiargs,"
41244100
"(%s proowner) AS rolname "
41254101
"FROM pg_proc p "
41264102
"WHERE NOT proisagg AND ("
@@ -4142,29 +4118,13 @@ getFuncs(Archive *fout, int *numFuncs)
41424118
"deptype = 'e')");
41434119
appendPQExpBuffer(query, ")");
41444120
}
4145-
else if (fout->remoteVersion >= 70300)
4146-
{
4147-
appendPQExpBuffer(query,
4148-
"SELECT tableoid, oid, proname, prolang, "
4149-
"pronargs, proargtypes, prorettype, proacl, "
4150-
"pronamespace, "
4151-
"NULL::text AS proiargs,"
4152-
"(%s proowner) AS rolname "
4153-
"FROM pg_proc p "
4154-
"WHERE NOT proisagg AND ("
4155-
"pronamespace != "
4156-
"(SELECT oid FROM pg_namespace "
4157-
"WHERE nspname = 'pg_catalog'))",
4158-
username_subquery);
4159-
}
41604121
else if (fout->remoteVersion >= 70100)
41614122
{
41624123
appendPQExpBuffer(query,
41634124
"SELECT tableoid, oid, proname, prolang, "
41644125
"pronargs, proargtypes, prorettype, "
41654126
"'{=X}' AS proacl, "
41664127
"0::oid AS pronamespace, "
4167-
"NULL::text AS proiargs,"
41684128
"(%s proowner) AS rolname "
41694129
"FROM pg_proc "
41704130
"WHERE pg_proc.oid > '%u'::oid",
@@ -4181,7 +4141,6 @@ getFuncs(Archive *fout, int *numFuncs)
41814141
"pronargs, proargtypes, prorettype, "
41824142
"'{=X}' AS proacl, "
41834143
"0::oid AS pronamespace, "
4184-
"NULL::text AS proiargs,"
41854144
"(%s proowner) AS rolname "
41864145
"FROM pg_proc "
41874146
"where pg_proc.oid > '%u'::oid",
@@ -4207,7 +4166,6 @@ getFuncs(Archive *fout, int *numFuncs)
42074166
i_proargtypes = PQfnumber(res, "proargtypes");
42084167
i_prorettype = PQfnumber(res, "prorettype");
42094168
i_proacl = PQfnumber(res, "proacl");
4210-
i_proiargs = PQfnumber(res, "proiargs");
42114169

42124170
for (i = 0; i < ntups; i++)
42134171
{
@@ -4223,7 +4181,6 @@ getFuncs(Archive *fout, int *numFuncs)
42234181
finfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
42244182
finfo[i].lang = atooid(PQgetvalue(res, i, i_prolang));
42254183
finfo[i].prorettype = atooid(PQgetvalue(res, i, i_prorettype));
4226-
finfo[i].proiargs = pg_strdup(PQgetvalue(res, i, i_proiargs));
42274184
finfo[i].proacl = pg_strdup(PQgetvalue(res, i, i_proacl));
42284185
finfo[i].nargs = atoi(PQgetvalue(res, i, i_pronargs));
42294186
if (finfo[i].nargs == 0)

‎src/bin/pg_dump/pg_dump.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@ typedef struct _funcInfo
184184
Oid *argtypes;
185185
Oid prorettype;
186186
char *proacl;
187-
char *proiargs;
188187
} FuncInfo;
189188

190189
/* AggInfo is a superset of FuncInfo */

‎src/bin/pg_dump/pg_dump_sort.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,13 +287,30 @@ DOTypeNameCompare(const void *p1, const void *p2)
287287
{
288288
FuncInfo *fobj1 = *(FuncInfo *const *) p1;
289289
FuncInfo *fobj2 = *(FuncInfo *const *) p2;
290+
int i;
290291

291292
cmpval = fobj1->nargs - fobj2->nargs;
292293
if (cmpval != 0)
293294
return cmpval;
294-
cmpval = strcmp(fobj1->proiargs, fobj2->proiargs);
295-
if (cmpval != 0)
296-
return cmpval;
295+
for (i = 0; i < fobj1->nargs; i++)
296+
{
297+
TypeInfo *argtype1 = findTypeByOid(fobj1->argtypes[i]);
298+
TypeInfo *argtype2 = findTypeByOid(fobj2->argtypes[i]);
299+
300+
if (argtype1 && argtype2)
301+
{
302+
if (argtype1->dobj.namespace && argtype2->dobj.namespace)
303+
{
304+
cmpval = strcmp(argtype1->dobj.namespace->dobj.name,
305+
argtype2->dobj.namespace->dobj.name);
306+
if (cmpval != 0)
307+
return cmpval;
308+
}
309+
cmpval = strcmp(argtype1->dobj.name, argtype2->dobj.name);
310+
if (cmpval != 0)
311+
return cmpval;
312+
}
313+
}
297314
}
298315
else if (obj1->objType == DO_OPERATOR)
299316
{

0 commit comments

Comments
 (0)