Joel Jacobson wrote:
> On Thu, Jul 5, 2012 at 10:33 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> 
> > You may in fact need a new field --- I'm just saying it should be in the
> > object-type-specific struct, eg FuncInfo, not DumpableObject.
> 
> 
> I suggest adding char *funcsig to FuncInfo, and moving the "funcsig =
> format_function_arguments(finfo, funciargs)" code from dumpFunc to getFuncs.
> 
> Because dumpFunc is called after sortDumpableObjectsByTypeName, setting
> funcsig in the FuncInfo struct in dumpFunc would't work, as it needs to be
> available when entering sortDumpableObjectsByTypeName.

Uh, the patch you posted keeps the pg_get_function_identity_arguments
call in dumpFunc, but there is now also a new one in getFuncs.  Do we
need to remove the second one?

Here's an updated patch for your consideration.  I was about to push
this when I noticed the above.  The only change here is that the extra
code that tests for new remoteVersions in the second "else if" branch of
getFuncs and getAggregates has been removed, since it cannot ever be
reached.

(I tested the new pg_dump with 8.2 and HEAD and also verified it passes
pg_upgrade's "make check".  I didn't test with other server versions.)

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** 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,3555 ----
  	 * 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 "
***************
*** 3565,3576 **** getAggregates(Archive *fout, int *numAggs)
--- 3567,3594 ----
  							  "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 "
+ 						  "WHERE proisagg AND ("
+ 						  "pronamespace != "
+ 						  "(SELECT oid FROM pg_namespace "
+ 						  "WHERE nspname = 'pg_catalog'))",
+ 						  username_subquery);
+ 	}
  	else if (fout->remoteVersion >= 70300)
  	{
  		appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
  						  "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)
--- 3603,3609 ----
  						  "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)
--- 3619,3625 ----
  						  "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)
--- 3643,3649 ----
  	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)
--- 3663,3669 ----
  		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)
--- 3715,3721 ----
  	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 ("
--- 3736,3748 ----
  	 * 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 ("
***************
*** 3740,3745 **** getFuncs(Archive *fout, int *numFuncs)
--- 3764,3784 ----
  							  "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 ("
+ 						  "pronamespace != "
+ 						  "(SELECT oid FROM pg_namespace "
+ 						  "WHERE nspname = 'pg_catalog'))",
+ 						  username_subquery);
+ 	}
  	else if (fout->remoteVersion >= 70100)
  	{
  		appendPQExpBuffer(query,
***************
*** 3747,3752 **** getFuncs(Archive *fout, int *numFuncs)
--- 3786,3792 ----
  						  "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)
--- 3803,3809 ----
  						  "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)
--- 3829,3835 ----
  	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)
--- 3845,3851 ----
  		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)
  	{
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to