Skip to content

P
Postgres FD Implementation
Commit e3bfe6d8 authored by Tom Lane's avatar Tom Lane
Browse files
Options

Rethink function argument sorting in pg_dump. 

Commit 7b583b20 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.
parent c6ee39bc
Hide whitespace changes
Inline Side-by-side
Showing with 22 additions and 49 deletions
src/bin/pg_dump/pg_dump.c
View file @ e3bfe6d8
...@@ -4199,7 +4199,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs) ...@@ -4199,7 +4199,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs)
int i_proargtypes; int i_proargtypes;
int i_rolname; int i_rolname;
int i_aggacl; int i_aggacl;
int i_proiargs;
/* Make sure we are in proper schema */ /* Make sure we are in proper schema */
selectSourceSchema(fout, "pg_catalog"); selectSourceSchema(fout, "pg_catalog");
...@@ -4209,12 +4208,11 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs) ...@@ -4209,12 +4208,11 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs)
* rationale behind the filtering logic. * rationale behind the filtering logic.
*/ */
if (fout->remoteVersion >= 80400) if (fout->remoteVersion >= 80200)
{ {
appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, " appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
"pronamespace AS aggnamespace, " "pronamespace AS aggnamespace, "
"pronargs, proargtypes, " "pronargs, proargtypes, "
"pg_catalog.pg_get_function_identity_arguments(oid) AS proiargs,"
"(%s proowner) AS rolname, " "(%s proowner) AS rolname, "
"proacl AS aggacl " "proacl AS aggacl "
"FROM pg_proc p " "FROM pg_proc p "
...@@ -4232,28 +4230,12 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs) ...@@ -4232,28 +4230,12 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs)
"deptype = 'e')"); "deptype = 'e')");
appendPQExpBufferChar(query, ')'); appendPQExpBufferChar(query, ')');
} }
else if (fout->remoteVersion >= 80200)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
"pronamespace AS aggnamespace, "
"pronargs, proargtypes, "
"NULL::text AS proiargs,"
"(%s proowner) AS rolname, "
"proacl AS aggacl "
"FROM pg_proc p "
"WHERE proisagg AND ("
"pronamespace != "
"(SELECT oid FROM pg_namespace "
"WHERE nspname = 'pg_catalog'))",
username_subquery);
}
else if (fout->remoteVersion >= 70300) else if (fout->remoteVersion >= 70300)
{ {
appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, " appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
"pronamespace AS aggnamespace, " "pronamespace AS aggnamespace, "
"CASE WHEN proargtypes[0] = 'pg_catalog.\"any\"'::pg_catalog.regtype THEN 0 ELSE 1 END AS pronargs, " "CASE WHEN proargtypes[0] = 'pg_catalog.\"any\"'::pg_catalog.regtype THEN 0 ELSE 1 END AS pronargs, "
"proargtypes, " "proargtypes, "
"NULL::text AS proiargs, "
"(%s proowner) AS rolname, " "(%s proowner) AS rolname, "
"proacl AS aggacl " "proacl AS aggacl "
"FROM pg_proc " "FROM pg_proc "
...@@ -4268,7 +4250,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs) ...@@ -4268,7 +4250,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs)
"0::oid AS aggnamespace, " "0::oid AS aggnamespace, "
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, " "CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
"aggbasetype AS proargtypes, " "aggbasetype AS proargtypes, "
"NULL::text AS proiargs, "
"(%s aggowner) AS rolname, " "(%s aggowner) AS rolname, "
"'{=X}' AS aggacl " "'{=X}' AS aggacl "
"FROM pg_aggregate " "FROM pg_aggregate "
...@@ -4284,7 +4265,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs) ...@@ -4284,7 +4265,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs)
"0::oid AS aggnamespace, " "0::oid AS aggnamespace, "
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, " "CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
"aggbasetype AS proargtypes, " "aggbasetype AS proargtypes, "
"NULL::text AS proiargs, "
"(%s aggowner) AS rolname, " "(%s aggowner) AS rolname, "
"'{=X}' AS aggacl " "'{=X}' AS aggacl "
"FROM pg_aggregate " "FROM pg_aggregate "
...@@ -4308,7 +4288,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs) ...@@ -4308,7 +4288,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs)
i_proargtypes = PQfnumber(res, "proargtypes"); i_proargtypes = PQfnumber(res, "proargtypes");
i_rolname = PQfnumber(res, "rolname"); i_rolname = PQfnumber(res, "rolname");
i_aggacl = PQfnumber(res, "aggacl"); i_aggacl = PQfnumber(res, "aggacl");
i_proiargs = PQfnumber(res, "proiargs");
for (i = 0; i < ntups; i++) for (i = 0; i < ntups; i++)
{ {
...@@ -4328,7 +4307,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs) ...@@ -4328,7 +4307,6 @@ getAggregates(Archive *fout, DumpOptions *dopt, int *numAggs)
agginfo[i].aggfn.lang = InvalidOid; /* not currently interesting */ agginfo[i].aggfn.lang = InvalidOid; /* not currently interesting */
agginfo[i].aggfn.prorettype = InvalidOid; /* not saved */ agginfo[i].aggfn.prorettype = InvalidOid; /* not saved */
agginfo[i].aggfn.proacl = pg_strdup(PQgetvalue(res, i, i_aggacl)); agginfo[i].aggfn.proacl = pg_strdup(PQgetvalue(res, i, i_aggacl));
agginfo[i].aggfn.proiargs = pg_strdup(PQgetvalue(res, i, i_proiargs));
agginfo[i].aggfn.nargs = atoi(PQgetvalue(res, i, i_pronargs)); agginfo[i].aggfn.nargs = atoi(PQgetvalue(res, i, i_pronargs));
if (agginfo[i].aggfn.nargs == 0) if (agginfo[i].aggfn.nargs == 0)
agginfo[i].aggfn.argtypes = NULL; agginfo[i].aggfn.argtypes = NULL;
...@@ -4380,7 +4358,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs) ...@@ -4380,7 +4358,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs)
int i_proargtypes; int i_proargtypes;
int i_prorettype; int i_prorettype;
int i_proacl; int i_proacl;
int i_proiargs;
/* Make sure we are in proper schema */ /* Make sure we are in proper schema */
selectSourceSchema(fout, "pg_catalog"); selectSourceSchema(fout, "pg_catalog");
...@@ -4401,13 +4378,12 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs) ...@@ -4401,13 +4378,12 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs)
* doesn't have; otherwise we might not get creation ordering correct. * doesn't have; otherwise we might not get creation ordering correct.
*/ */
if (fout->remoteVersion >= 80400) if (fout->remoteVersion >= 70300)
{ {
appendPQExpBuffer(query, appendPQExpBuffer(query,
"SELECT tableoid, oid, proname, prolang, " "SELECT tableoid, oid, proname, prolang, "
"pronargs, proargtypes, prorettype, proacl, " "pronargs, proargtypes, prorettype, proacl, "
"pronamespace, " "pronamespace, "
"pg_catalog.pg_get_function_identity_arguments(oid) AS proiargs,"
"(%s proowner) AS rolname " "(%s proowner) AS rolname "
"FROM pg_proc p " "FROM pg_proc p "
"WHERE NOT proisagg AND (" "WHERE NOT proisagg AND ("
...@@ -4429,21 +4405,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs) ...@@ -4429,21 +4405,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs)
"deptype = 'e')"); "deptype = 'e')");
appendPQExpBufferChar(query, ')'); appendPQExpBufferChar(query, ')');
} }
else if (fout->remoteVersion >= 70300)
{
appendPQExpBuffer(query,
"SELECT tableoid, oid, proname, prolang, "
"pronargs, proargtypes, prorettype, proacl, "
"pronamespace, "
"NULL::text AS proiargs,"
"(%s proowner) AS rolname "
"FROM pg_proc p "
"WHERE NOT proisagg AND ("
"pronamespace != "
"(SELECT oid FROM pg_namespace "
"WHERE nspname = 'pg_catalog'))",
username_subquery);
}
else if (fout->remoteVersion >= 70100) else if (fout->remoteVersion >= 70100)
{ {
appendPQExpBuffer(query, appendPQExpBuffer(query,
...@@ -4451,7 +4412,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs) ...@@ -4451,7 +4412,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs)
"pronargs, proargtypes, prorettype, " "pronargs, proargtypes, prorettype, "
"'{=X}' AS proacl, " "'{=X}' AS proacl, "
"0::oid AS pronamespace, " "0::oid AS pronamespace, "
"NULL::text AS proiargs,"
"(%s proowner) AS rolname " "(%s proowner) AS rolname "
"FROM pg_proc " "FROM pg_proc "
"WHERE pg_proc.oid > '%u'::oid", "WHERE pg_proc.oid > '%u'::oid",
...@@ -4468,7 +4428,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs) ...@@ -4468,7 +4428,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs)
"pronargs, proargtypes, prorettype, " "pronargs, proargtypes, prorettype, "
"'{=X}' AS proacl, " "'{=X}' AS proacl, "
"0::oid AS pronamespace, " "0::oid AS pronamespace, "
"NULL::text AS proiargs,"
"(%s proowner) AS rolname " "(%s proowner) AS rolname "
"FROM pg_proc " "FROM pg_proc "
"where pg_proc.oid > '%u'::oid", "where pg_proc.oid > '%u'::oid",
...@@ -4494,7 +4453,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs) ...@@ -4494,7 +4453,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs)
i_proargtypes = PQfnumber(res, "proargtypes"); i_proargtypes = PQfnumber(res, "proargtypes");
i_prorettype = PQfnumber(res, "prorettype"); i_prorettype = PQfnumber(res, "prorettype");
i_proacl = PQfnumber(res, "proacl"); i_proacl = PQfnumber(res, "proacl");
i_proiargs = PQfnumber(res, "proiargs");
for (i = 0; i < ntups; i++) for (i = 0; i < ntups; i++)
{ {
...@@ -4510,7 +4468,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs) ...@@ -4510,7 +4468,6 @@ getFuncs(Archive *fout, DumpOptions *dopt, int *numFuncs)
finfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname)); finfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
finfo[i].lang = atooid(PQgetvalue(res, i, i_prolang)); finfo[i].lang = atooid(PQgetvalue(res, i, i_prolang));
finfo[i].prorettype = atooid(PQgetvalue(res, i, i_prorettype)); finfo[i].prorettype = atooid(PQgetvalue(res, i, i_prorettype));
finfo[i].proiargs = pg_strdup(PQgetvalue(res, i, i_proiargs));
finfo[i].proacl = pg_strdup(PQgetvalue(res, i, i_proacl)); finfo[i].proacl = pg_strdup(PQgetvalue(res, i, i_proacl));
finfo[i].nargs = atoi(PQgetvalue(res, i, i_pronargs)); finfo[i].nargs = atoi(PQgetvalue(res, i, i_pronargs));
if (finfo[i].nargs == 0) if (finfo[i].nargs == 0)
......
src/bin/pg_dump/pg_dump.h
View file @ e3bfe6d8
...@@ -149,7 +149,6 @@ typedef struct _funcInfo ...@@ -149,7 +149,6 @@ typedef struct _funcInfo
Oid *argtypes; Oid *argtypes;
Oid prorettype; Oid prorettype;
char *proacl; char *proacl;
char *proiargs;
} FuncInfo; } FuncInfo;
/* AggInfo is a superset of FuncInfo */ /* AggInfo is a superset of FuncInfo */
......
src/bin/pg_dump/pg_dump_sort.c
View file @ e3bfe6d8
...@@ -291,13 +291,30 @@ DOTypeNameCompare(const void *p1, const void *p2) ...@@ -291,13 +291,30 @@ DOTypeNameCompare(const void *p1, const void *p2)
{ {
FuncInfo *fobj1 = *(FuncInfo *const *) p1; FuncInfo *fobj1 = *(FuncInfo *const *) p1;
FuncInfo *fobj2 = *(FuncInfo *const *) p2; FuncInfo *fobj2 = *(FuncInfo *const *) p2;
int i;
cmpval = fobj1->nargs - fobj2->nargs; cmpval = fobj1->nargs - fobj2->nargs;
if (cmpval != 0) if (cmpval != 0)
return cmpval; return cmpval;
cmpval = strcmp(fobj1->proiargs, fobj2->proiargs); for (i = 0; i < fobj1->nargs; i++)
if (cmpval != 0) {
return cmpval; TypeInfo *argtype1 = findTypeByOid(fobj1->argtypes[i]);
TypeInfo *argtype2 = findTypeByOid(fobj2->argtypes[i]);
if (argtype1 && argtype2)
{
if (argtype1->dobj.namespace && argtype2->dobj.namespace)
{
cmpval = strcmp(argtype1->dobj.namespace->dobj.name,
argtype2->dobj.namespace->dobj.name);
if (cmpval != 0)
return cmpval;
}
cmpval = strcmp(argtype1->dobj.name, argtype2->dobj.name);
if (cmpval != 0)
return cmpval;
}
}
} }
else if (obj1->objType == DO_OPERATOR) else if (obj1->objType == DO_OPERATOR)
{ {
......
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