Обсуждение: Bug with ordering aggregates?

Greetings,
 This doesn't seem right to me:

postgres=# select
postgres-# string_agg(column1::text order by column1 asc,',')
postgres-# from (values (3),(4),(1),(2)) a;string_agg
------------1234
(1 row)
 I'm thinking we should toss a syntax error here and force the 'order by' to be at the end of any arguments to the
aggregate.Alternatively, we should actually make this work like this one does: 

postgres=# select
postgres-# string_agg(column1::text,',' order by column1 asc)
postgres-# from (values (3),(4),(1),(2)) a;string_agg
------------1,2,3,4
(1 row)
Thanks,
    Stephen

Stephen Frost <sfrost@snowman.net> writes:
>   This doesn't seem right to me:

> postgres=# select
> postgres-# string_agg(column1::text order by column1 asc,',')
> postgres-# from (values (3),(4),(1),(2)) a;
>  string_agg 
> ------------
>  1234
> (1 row)

Looks fine to me: you have two ordering columns (the second rather
useless,  but that's no matter).
        regards, tom lane

On 18 May 2010 16:37, Stephen Frost <sfrost@snowman.net> wrote:
> Greetings,
>
>  This doesn't seem right to me:
>
> postgres=# select
> postgres-# string_agg(column1::text order by column1 asc,',')
> postgres-# from (values (3),(4),(1),(2)) a;
>  string_agg
> ------------
>  1234
> (1 row)
>
>  I'm thinking we should toss a syntax error here and force the 'order
>  by' to be at the end of any arguments to the aggregate.
>  Alternatively, we should actually make this work like this one does:
>
> postgres=# select
> postgres-# string_agg(column1::text,',' order by column1 asc)
> postgres-# from (values (3),(4),(1),(2)) a;
>  string_agg
> ------------
>  1,2,3,4
> (1 row)
>

I find that 2nd example confusing.  It suggests the delimiter is being
ordered as the order by clause appears in its parameter.  But I can
see why the first one is returning the wrong result.  The order by
clause conflicts with the delimiter parameter as obviously the order
by clause prevents you specifying a 2nd parameter in the aggregate
function.  The delimiter would either need to be the first parameter,
or the order by clause would require a way to terminate it's order by
list.

Thom

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> >   This doesn't seem right to me:
>
> > postgres=# select
> > postgres-# string_agg(column1::text order by column1 asc,',')
> > postgres-# from (values (3),(4),(1),(2)) a;
> >  string_agg
> > ------------
> >  1234
> > (1 row)
>
> Looks fine to me: you have two ordering columns (the second rather
> useless,  but that's no matter).

Ah, yeah, guess I'll just complain that having the order by look like
it's an argument to an aggregate makes things confusing.  Not much to be
done about it though.
Thanks,
    Stephen

This is an area that the SQL standard didn't think through very clearly
(IMHO).  They actually have two ways of specifying functions like this, one
is the ordered aggregate section that this syntax is modeled on, which is
indeed very confusing for multi-parameter aggregates.  The other is the
hypothetical set function syntax which is actually much clearer for this
sort of operation, though I haven't dug deep enough into the standard to be
sure this wouldn't include any gotchas:
 SELECT agg(parameter1, parameter2) WITHIN GROUP (ORDER BY column1 asc)

(See section 10.9 on <aggregate function> syntax)

Supporting the hypthothetical set functions could give a preferable syntax.

Regards, Caleb

On 5/18/10 9:42 AM, "Stephen Frost" <sfrost@snowman.net> wrote:

> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> Stephen Frost <sfrost@snowman.net> writes:
>>>   This doesn't seem right to me:
>>
>>> postgres=# select
>>> postgres-# string_agg(column1::text order by column1 asc,',')
>>> postgres-# from (values (3),(4),(1),(2)) a;
>>>  string_agg
>>> ------------
>>>  1234
>>> (1 row)
>>
>> Looks fine to me: you have two ordering columns (the second rather
>> useless,  but that's no matter).
>
> Ah, yeah, guess I'll just complain that having the order by look like
> it's an argument to an aggregate makes things confusing.  Not much to be
> done about it though.
>
> Thanks,
>
> Stephen