Re: Review for pg_dump: Sort overloaded functions in deterministic order
Started by Joel Jacobsonover 13 years ago2 messages
Hi Joachim,
Attached, please find new patch. Test unchanged.
Best regards,
Joel
Show quoted text
Patch looks good, all concerns that had been raised previously have
been addressed in this version of the patch.The only thing that IMO needs to change is a stylistic issue:
if (fout->remoteVersion >= 80200)
{
[...]
(fout->remoteVersion >= 80400) ?
"pg_catalog.pg_get_function_identity_arguments(oid)" : "NULL::text",
[...]
}Please just create a whole new
if (fout->remoteVersion >= 80400)
{
[...]
}here.
Other than that, the feature works as advertised and does not
negatively affect runtime or memory consumption (the new field is only
added to functions / aggregates).Please send a new version of the patch that changes the above
mentioned item, the patch also needs rebasing (off by a couple of
lines).
Attachments:
pg_dump_deterministic_order_v5.patchapplication/octet-stream; name=pg_dump_deterministic_order_v5.patchDownload
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
***************
*** 3534,3539 **** getAggregates(Archive *fout, int *numAggs)
--- 3534,3540 ----
int i_proargtypes;
int i_rolname;
int i_aggacl;
+ int i_proiargs;
/* Make sure we are in proper schema */
selectSourceSchema(fout, "pg_catalog");
***************
*** 3543,3553 **** getAggregates(Archive *fout, int *numAggs)
* rationale behind the filtering logic.
*/
! if (fout->remoteVersion >= 80200)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
"pronamespace AS aggnamespace, "
"pronargs, proargtypes, "
"(%s proowner) AS rolname, "
"proacl AS aggacl "
"FROM pg_proc p "
--- 3544,3578 ----
* rationale behind the filtering logic.
*/
! if (fout->remoteVersion >= 80400)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
"pronamespace AS aggnamespace, "
"pronargs, proargtypes, "
+ "pg_catalog.pg_get_function_identity_arguments(oid) 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);
+ if (binary_upgrade && fout->remoteVersion >= 90100)
+ appendPQExpBuffer(query,
+ " OR EXISTS(SELECT 1 FROM pg_depend WHERE "
+ "classid = 'pg_proc'::regclass AND "
+ "objid = p.oid AND "
+ "refclassid = 'pg_extension'::regclass AND "
+ "deptype = 'e')");
+ appendPQExpBuffer(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 "
***************
*** 3571,3576 **** getAggregates(Archive *fout, int *numAggs)
--- 3596,3602 ----
"pronamespace AS aggnamespace, "
"CASE WHEN proargtypes[0] = 'pg_catalog.\"any\"'::pg_catalog.regtype THEN 0 ELSE 1 END AS pronargs, "
"proargtypes, "
+ "NULL::text AS proiargs, "
"(%s proowner) AS rolname, "
"proacl AS aggacl "
"FROM pg_proc "
***************
*** 3585,3590 **** getAggregates(Archive *fout, int *numAggs)
--- 3611,3617 ----
"0::oid AS aggnamespace, "
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
"aggbasetype AS proargtypes, "
+ "NULL::text AS proiargs, "
"(%s aggowner) AS rolname, "
"'{=X}' AS aggacl "
"FROM pg_aggregate "
***************
*** 3600,3605 **** getAggregates(Archive *fout, int *numAggs)
--- 3627,3633 ----
"0::oid AS aggnamespace, "
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
"aggbasetype AS proargtypes, "
+ "NULL::text AS proiargs, "
"(%s aggowner) AS rolname, "
"'{=X}' AS aggacl "
"FROM pg_aggregate "
***************
*** 3623,3628 **** getAggregates(Archive *fout, int *numAggs)
--- 3651,3657 ----
i_proargtypes = PQfnumber(res, "proargtypes");
i_rolname = PQfnumber(res, "rolname");
i_aggacl = PQfnumber(res, "aggacl");
+ i_proiargs = PQfnumber(res, "proiargs");
for (i = 0; i < ntups; i++)
{
***************
*** 3642,3647 **** getAggregates(Archive *fout, int *numAggs)
--- 3671,3677 ----
agginfo[i].aggfn.lang = InvalidOid; /* not currently interesting */
agginfo[i].aggfn.prorettype = InvalidOid; /* not saved */
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));
if (agginfo[i].aggfn.nargs == 0)
agginfo[i].aggfn.argtypes = NULL;
***************
*** 3693,3698 **** getFuncs(Archive *fout, int *numFuncs)
--- 3723,3729 ----
int i_proargtypes;
int i_prorettype;
int i_proacl;
+ int i_proiargs;
/* Make sure we are in proper schema */
selectSourceSchema(fout, "pg_catalog");
***************
*** 3713,3724 **** getFuncs(Archive *fout, int *numFuncs)
* doesn't have; otherwise we might not get creation ordering correct.
*/
! if (fout->remoteVersion >= 70300)
{
appendPQExpBuffer(query,
"SELECT tableoid, oid, proname, prolang, "
"pronargs, proargtypes, prorettype, proacl, "
"pronamespace, "
"(%s proowner) AS rolname "
"FROM pg_proc p "
"WHERE NOT proisagg AND ("
--- 3744,3784 ----
* doesn't have; otherwise we might not get creation ordering correct.
*/
! if (fout->remoteVersion >= 80400)
! {
! appendPQExpBuffer(query,
! "SELECT tableoid, oid, proname, prolang, "
! "pronargs, proargtypes, prorettype, proacl, "
! "pronamespace, "
! "pg_catalog.pg_get_function_identity_arguments(oid) 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);
! if (fout->remoteVersion >= 90200)
! appendPQExpBuffer(query,
! "\n AND NOT EXISTS (SELECT 1 FROM pg_depend "
! "WHERE classid = 'pg_proc'::regclass AND "
! "objid = p.oid AND deptype = 'i')");
! if (binary_upgrade && fout->remoteVersion >= 90100)
! appendPQExpBuffer(query,
! "\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
! "classid = 'pg_proc'::regclass AND "
! "objid = p.oid AND "
! "refclassid = 'pg_extension'::regclass AND "
! "deptype = 'e')");
! appendPQExpBuffer(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 ("
***************
*** 3747,3752 **** getFuncs(Archive *fout, int *numFuncs)
--- 3807,3813 ----
"pronargs, proargtypes, prorettype, "
"'{=X}' AS proacl, "
"0::oid AS pronamespace, "
+ "NULL::text AS proiargs,"
"(%s proowner) AS rolname "
"FROM pg_proc "
"WHERE pg_proc.oid > '%u'::oid",
***************
*** 3763,3768 **** getFuncs(Archive *fout, int *numFuncs)
--- 3824,3830 ----
"pronargs, proargtypes, prorettype, "
"'{=X}' AS proacl, "
"0::oid AS pronamespace, "
+ "NULL::text AS proiargs,"
"(%s proowner) AS rolname "
"FROM pg_proc "
"where pg_proc.oid > '%u'::oid",
***************
*** 3788,3793 **** getFuncs(Archive *fout, int *numFuncs)
--- 3850,3856 ----
i_proargtypes = PQfnumber(res, "proargtypes");
i_prorettype = PQfnumber(res, "prorettype");
i_proacl = PQfnumber(res, "proacl");
+ i_proiargs = PQfnumber(res, "proiargs");
for (i = 0; i < ntups; i++)
{
***************
*** 3803,3808 **** getFuncs(Archive *fout, int *numFuncs)
--- 3866,3872 ----
finfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
finfo[i].lang = atooid(PQgetvalue(res, i, i_prolang));
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].nargs = atoi(PQgetvalue(res, i, i_pronargs));
if (finfo[i].nargs == 0)
*** a/src/bin/pg_dump/pg_dump.h
--- b/src/bin/pg_dump/pg_dump.h
***************
*** 193,198 **** typedef struct _funcInfo
--- 193,199 ----
Oid *argtypes;
Oid prorettype;
char *proacl;
+ char *proiargs;
} FuncInfo;
/* AggInfo is a superset of FuncInfo */
*** a/src/bin/pg_dump/pg_dump_sort.c
--- b/src/bin/pg_dump/pg_dump_sort.c
***************
*** 198,203 **** DOTypeNameCompare(const void *p1, const void *p2)
--- 198,206 ----
cmpval = fobj1->nargs - fobj2->nargs;
if (cmpval != 0)
return cmpval;
+ cmpval = strcmp(fobj1->proiargs, fobj2->proiargs);
+ if (cmpval != 0)
+ return cmpval;
}
else if (obj1->objType == DO_OPERATOR)
{
Re: Review for pg_dump: Sort overloaded functions in deterministic order
Joel Jacobson wrote:
Hi Joachim,
Attached, please find new patch. Test unchanged.
This was committed, as discussed in the original patch's thread.
It would be great if reviewers could reply to the email that submits the
patch, instead of creating a thread of their own. It helps keep things
better organized.
Thanks for the review.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services