Обсуждение: pg_dump sort order for functions

pg_dump sorts its output first by object type, then by object name, and
then processes all that for dependencies.  This works well, but for
overloaded functions this still gives a random sort order that can
produce annoying diffs in the dump.

Would it be acceptable to introduce a secondary sort key field into the
DumpableObject struct that functions would fill with, say, the argument
types (maybe something like "text,int,int" -- need to play with this a
little)?

Peter Eisentraut <peter_e@gmx.net> writes:
> pg_dump sorts its output first by object type, then by object name, and
> then processes all that for dependencies.  This works well, but for
> overloaded functions this still gives a random sort order that can
> produce annoying diffs in the dump.

> Would it be acceptable to introduce a secondary sort key field into the
> DumpableObject struct that functions would fill with, say, the argument
> types (maybe something like "text,int,int" -- need to play with this a
> little)?

I think you could probably use the existing tag field; no need for a new
one.  The real problem is that an object-type-specific sort rule is a
bit klugy.  IIRC there is discussion of this in the archives already ...
        regards, tom lane

On mån, 2010-01-11 at 10:44 -0500, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > pg_dump sorts its output first by object type, then by object name, and
> > then processes all that for dependencies.  This works well, but for
> > overloaded functions this still gives a random sort order that can
> > produce annoying diffs in the dump.
> 
> > Would it be acceptable to introduce a secondary sort key field into the
> > DumpableObject struct that functions would fill with, say, the argument
> > types (maybe something like "text,int,int" -- need to play with this a
> > little)?
> 
> I think you could probably use the existing tag field; no need for a new
> one.

Sorry, which tag field are you referring to?

Peter Eisentraut <peter_e@gmx.net> writes:
> On mån, 2010-01-11 at 10:44 -0500, Tom Lane wrote:
>> I think you could probably use the existing tag field; no need for a new
>> one.

> Sorry, which tag field are you referring to?

The one called "tag" in the source code.  It prints out as "Name":

--
-- Name: binary_coercible(oid, oid); Type: FUNCTION; Schema: public; Owner: postgres        ^^^^^^^^^^^^^^^^^^^^^^^^^^
--
        regards, tom lane

On mån, 2010-01-11 at 12:54 -0500, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > On mån, 2010-01-11 at 10:44 -0500, Tom Lane wrote:
> >> I think you could probably use the existing tag field; no need for a new
> >> one.
>
> > Sorry, which tag field are you referring to?
>
> The one called "tag" in the source code.  It prints out as "Name":
>
> --
> -- Name: binary_coercible(oid, oid); Type: FUNCTION; Schema: public; Owner: postgres
>          ^^^^^^^^^^^^^^^^^^^^^^^^^^
> --

Um, that tag is the "name", and if you change that, the name in CREATE
FUNCTION also changes.  I was initially thinking in that direction, but
it seems it won't be feasible without significant refactoring.

In the mean time, hacking it into the sort function itself as a special
case works out fine, per attached patch.  One might frown upon such an
exception, but then again, function overloading is an exception to the
one-name-per-object rule all over the place anyway. ;-)

Peter Eisentraut <peter_e@gmx.net> writes:
> On mån, 2010-01-11 at 12:54 -0500, Tom Lane wrote:
>> -- Name: binary_coercible(oid, oid); Type: FUNCTION; Schema: public; Owner: postgres

> Um, that tag is the "name", and if you change that, the name in CREATE
> FUNCTION also changes.

So?

> In the mean time, hacking it into the sort function itself as a special
> case works out fine, per attached patch.  One might frown upon such an
> exception, but then again, function overloading is an exception to the
> one-name-per-object rule all over the place anyway. ;-)

No, that's a completely bogus solution, because it depends on type
OIDs.  It won't be stable across dump/reload, which defeats the purpose
AFAICS.
        regards, tom lane

On Tue, Jan 12, 2010 at 15:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> On mån, 2010-01-11 at 12:54 -0500, Tom Lane wrote:
>>> -- Name: binary_coercible(oid, oid); Type: FUNCTION; Schema: public; Owner: postgres
>
>> Um, that tag is the "name", and if you change that, the name in CREATE
>> FUNCTION also changes.
>
> So?

Sorry if this is talking about something completely different, haven't
followed the thread closely, but: will this change the output of
pg_restore -l? If so, changing the tag is likely to break scripts, and
IMHO should be avoided if possible.


-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus@hagander.net> writes:
> Sorry if this is talking about something completely different, haven't
> followed the thread closely, but: will this change the output of
> pg_restore -l? If so, changing the tag is likely to break scripts, and
> IMHO should be avoided if possible.

Only to the extent of possibly changing the order of entries.  Neither
of us is proposing changing the content of the tag.
        regards, tom lane

On Tue, Jan 12, 2010 at 15:59, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Sorry if this is talking about something completely different, haven't
>> followed the thread closely, but: will this change the output of
>> pg_restore -l? If so, changing the tag is likely to break scripts, and
>> IMHO should be avoided if possible.
>
> Only to the extent of possibly changing the order of entries.  Neither
> of us is proposing changing the content of the tag.

Ok, then I think it'd be fine. Ignore me :-)


-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/

I wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> On mån, 2010-01-11 at 12:54 -0500, Tom Lane wrote:
>>> -- Name: binary_coercible(oid, oid); Type: FUNCTION; Schema: public; Owner: postgres

>> Um, that tag is the "name", and if you change that, the name in CREATE
>> FUNCTION also changes.

> So?

Actually, we're talking at cross-purposes here.  The tag I'm talking
about is the one generated via format_function_signature, and the
problem with what I had in mind is that it isn't done yet at the point
where the sort runs.

>> In the mean time, hacking it into the sort function itself as a special
>> case works out fine, per attached patch.  One might frown upon such an
>> exception, but then again, function overloading is an exception to the
>> one-name-per-object rule all over the place anyway. ;-)

> No, that's a completely bogus solution, because it depends on type
> OIDs.  It won't be stable across dump/reload, which defeats the purpose
> AFAICS.

You could probably make it work more safely if you applied
getFormattedTypeName() and then compared the string names.
That would be rather expensive :-( but in most databases
this should happen few enough times so it wouldn't be a problem.

[ thinks for a bit ... ]  Although getFormattedTypeName depends on
the current search_path, so that might be a bit of an issue for
stability as well.  I guess we could force a standardized path,
perhaps pg_catalog only, before sorting.
        regards, tom lane

On tis, 2010-01-12 at 16:35 +0200, Peter Eisentraut wrote:
> Um, that tag is the "name", and if you change that, the name in CREATE
> FUNCTION also changes.  I was initially thinking in that direction, but
> it seems it won't be feasible without significant refactoring.
> 
> In the mean time, hacking it into the sort function itself as a special
> case works out fine, per attached patch.  One might frown upon such an
> exception, but then again, function overloading is an exception to the
> one-name-per-object rule all over the place anyway. ;-)

Since we ran out of time/ideas on this, I would propose just committing
the part that breaks ties based on the number of arguments, which
already solves a large part of the problem (at least in a pre-default
values world) and would very likely be a part of any possible future
utterly complete solution.

Peter Eisentraut <peter_e@gmx.net> writes:
> Since we ran out of time/ideas on this, I would propose just committing
> the part that breaks ties based on the number of arguments, which
> already solves a large part of the problem (at least in a pre-default
> values world) and would very likely be a part of any possible future
> utterly complete solution.

Seems safe enough.
        regards, tom lane