Re: [HACKERS] Rethinking pg_dump's function sorting code

2015-03-06 Thread Tom Lane
Noah Misch n...@leadboat.com writes:
 Comparing argument type names sounds fine.  Comparing argument type OID does
 not offer enough to justify the loss of cross-cluster sort equivalence.

Fair enough.

 So as to stably compare f(nsp0.t) to f(nsp1.t), this should also compare the
 dobj.namespace name.

Ah, good point.  Will fix.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Rethinking pg_dump's function sorting code

2015-03-06 Thread Marko Tiikkaja

On 2015-03-06 01:28, Tom Lane wrote:

In bug #12832 Marko Tiikkaja points out that commit
7b583b20b1c95acb621c71251150beef958bb603 created a rather unnecessary
dump failure hazard, since it applies pg_get_function_identity_arguments()
to every function in the database, even those that won't get dumped.
I think we should fix this by getting rid of pg_dump's use of that
function altogether.  A low-tech way to sort functions of identical names
would be to compare argument type OIDs, as in the attached simple patch.
If people feel it's important to avoid depending on numerical OID order,
we could instead look up type names locally and compare them as in the
attached less-simple patch.  (Both patches neglect reverting the data
collection aspects of the prior commit, since that's mechanical; the only
interesting part is what we'll do to sort.)

Neither patch will exactly preserve the sort behavior of the current
code, but I don't think that's important.

Comments?


I have my own cow in this ditch, but I would much prefer the sort to be 
done based on the type name.  That way the order is still consistent 
between two databases where the objects were created in a different order.



.m


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Rethinking pg_dump's function sorting code

2015-03-06 Thread Noah Misch
On Thu, Mar 05, 2015 at 07:28:33PM -0500, Tom Lane wrote:
 In bug #12832 Marko Tiikkaja points out that commit
 7b583b20b1c95acb621c71251150beef958bb603 created a rather unnecessary
 dump failure hazard, since it applies pg_get_function_identity_arguments()
 to every function in the database, even those that won't get dumped.
 I think we should fix this by getting rid of pg_dump's use of that
 function altogether.  A low-tech way to sort functions of identical names
 would be to compare argument type OIDs, as in the attached simple patch.
 If people feel it's important to avoid depending on numerical OID order,
 we could instead look up type names locally and compare them as in the
 attached less-simple patch.

Comparing argument type names sounds fine.  Comparing argument type OID does
not offer enough to justify the loss of cross-cluster sort equivalence.

 Neither patch will exactly preserve the sort behavior of the current
 code, but I don't think that's important.

Agreed.

 --- 291,313 
   {
   FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
   FuncInfo   *fobj2 = *(FuncInfo *const *) p2;
 + int i;
   
   cmpval = fobj1-nargs - fobj2-nargs;
   if (cmpval != 0)
   return cmpval;
 ! for (i = 0; i  fobj1-nargs; i++)
 ! {
 ! TypeInfo   *argtype1 = 
 findTypeByOid(fobj1-argtypes[i]);
 ! TypeInfo   *argtype2 = 
 findTypeByOid(fobj2-argtypes[i]);
 ! 
 ! if (argtype1  argtype2)
 ! {
 ! cmpval = strcmp(argtype1-dobj.name, 
 argtype2-dobj.name);
 ! if (cmpval != 0)
 ! return cmpval;
 ! }
 ! }

So as to stably compare f(nsp0.t) to f(nsp1.t), this should also compare the
dobj.namespace name.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Rethinking pg_dump's function sorting code

2015-03-05 Thread Tom Lane
In bug #12832 Marko Tiikkaja points out that commit
7b583b20b1c95acb621c71251150beef958bb603 created a rather unnecessary
dump failure hazard, since it applies pg_get_function_identity_arguments()
to every function in the database, even those that won't get dumped.
I think we should fix this by getting rid of pg_dump's use of that
function altogether.  A low-tech way to sort functions of identical names
would be to compare argument type OIDs, as in the attached simple patch.
If people feel it's important to avoid depending on numerical OID order,
we could instead look up type names locally and compare them as in the
attached less-simple patch.  (Both patches neglect reverting the data
collection aspects of the prior commit, since that's mechanical; the only
interesting part is what we'll do to sort.)

Neither patch will exactly preserve the sort behavior of the current
code, but I don't think that's important.

Comments?

regards, tom lane

diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 4b9bba0..816c9f0 100644
*** a/src/bin/pg_dump/pg_dump_sort.c
--- b/src/bin/pg_dump/pg_dump_sort.c
*** DOTypeNameCompare(const void *p1, const 
*** 291,303 
  	{
  		FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
  		FuncInfo   *fobj2 = *(FuncInfo *const *) p2;
  
  		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)
  	{
--- 291,307 
  	{
  		FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
  		FuncInfo   *fobj2 = *(FuncInfo *const *) p2;
+ 		int			i;
  
  		cmpval = fobj1-nargs - fobj2-nargs;
  		if (cmpval != 0)
  			return cmpval;
! 		for (i = 0; i  fobj1-nargs; i++)
! 		{
! 			cmpval = oidcmp(fobj1-argtypes[i], fobj2-argtypes[i]);
! 			if (cmpval != 0)
! return cmpval;
! 		}
  	}
  	else if (obj1-objType == DO_OPERATOR)
  	{
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 4b9bba0..de9407c 100644
*** a/src/bin/pg_dump/pg_dump_sort.c
--- b/src/bin/pg_dump/pg_dump_sort.c
*** DOTypeNameCompare(const void *p1, const 
*** 291,303 
  	{
  		FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
  		FuncInfo   *fobj2 = *(FuncInfo *const *) p2;
  
  		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)
  	{
--- 291,313 
  	{
  		FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
  		FuncInfo   *fobj2 = *(FuncInfo *const *) p2;
+ 		int			i;
  
  		cmpval = fobj1-nargs - fobj2-nargs;
  		if (cmpval != 0)
  			return cmpval;
! 		for (i = 0; i  fobj1-nargs; i++)
! 		{
! 			TypeInfo   *argtype1 = findTypeByOid(fobj1-argtypes[i]);
! 			TypeInfo   *argtype2 = findTypeByOid(fobj2-argtypes[i]);
! 
! 			if (argtype1  argtype2)
! 			{
! cmpval = strcmp(argtype1-dobj.name, argtype2-dobj.name);
! 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