Re: Review for pg_dump: Sort overloaded functions in deterministic order

Lists: pgsql-hackers
From: Joel Jacobson <joel(at)trustly(dot)com>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Joachim Wieland <joe(at)mcknight(dot)de>
Subject: Re: Review for pg_dump: Sort overloaded functions in deterministic order
Date: 2012-10-10 21:07:34
Message-ID: CAASwCXcHgEoM7CiOahL54GEMDpyuUU81nEOpXdCdn60_+UZ5VA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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

Attachment Content-Type Size
pg_dump_deterministic_order_v5.patch application/octet-stream 8.4 KB
pg_dump_deterministic_order.t application/x-troff 1.2 KB
From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Joel Jacobson <joel(at)trustly(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Joachim Wieland <joe(at)mcknight(dot)de>
Subject: Re: Review for pg_dump: Sort overloaded functions in deterministic order
Date: 2012-10-18 15:29:52
Message-ID: 20121018152952.GG1982@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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