Re: [HACKERS] Review for pg_dump: Sort overloaded functions in deterministic order

2012-10-18 Thread Alvaro Herrera 
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 Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] Review for pg_dump: Sort overloaded functions in deterministic order

2012-10-10 Thread Joel Jacobson 
Hi Joachim,

Attached, please find new patch. Test unchanged.

Best regards,

Joel

 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).


pg_dump_deterministic_order_v5.patch
Description: Binary data


pg_dump_deterministic_order.t
Description: Troff document

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

[HACKERS] Review for pg_dump: Sort overloaded functions in deterministic order

2012-09-25 Thread Joachim Wieland 
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).


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