Re: Review: listagg aggregate

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, "David E(dot) Wheeler" <david(at)kineticode(dot)com>, "pgsql-hackers(at)postgresql(dot)org Hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Review: listagg aggregate
Date: 2010-01-28 15:56:45
Message-ID: 15536.1264694205@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> simplest could not be a best. There have to be only a const
>> expression. But we have not possibility to check it in pg.

> Well... that's an entirely arbitrary limitation. I admit that it
> doesn't seem likely that someone would want to have a variable
> delimiter, but putting extra effort and code complexity into
> preventing it seems pointless.

Yeah. The real issue here is that in some cases you'd like to have
non-aggregated parameters to an aggregate, but SQL has no notation
to express that.

I think Pavel's underlying complaint is that if the delimiter
argument isn't constant, then we're exposing an implementation
dependency in terms of just which values get separated by which
delimiters. The most practical implementation seems to be that
the first-call delimiter isn't actually used at all, and on
subsequent calls the delimiter *precedes* the associated value,
which is a bit surprising given the order in which one writes
them. Not sure if this is worth documenting though. Those two
or three people who actually try it will figure it out soon enough.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2010-01-28 16:01:23 Re: Review: listagg aggregate
Previous Message Magnus Hagander 2010-01-28 15:53:44 Re: pgsql: Define INADDR_NONE on Solaris when it's missing.