Re: Aggregate error message

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Aggregate error message
Date: 2019-05-24 13:40:28
Message-ID: 4100.1558705228@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> writes:
> On Fri, 24 May 2019 at 18:17, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> wrote:
>> With a sample query such as
>> SELECT x, avg(x)
>> FROM (VALUES (1), (2), (3)) AS v (x);
>> We give the error message "column "v.x" must appear in the GROUP BY
>> clause or be used in an aggregate function".
>> This is correct but incomplete. Attached is a trivial patch to also
>> suggest that the user might have been trying to use a window function.

> I think you might have misthought this one. If there's an aggregate
> function in the SELECT or HAVING clause, then anything else in the
> SELECT clause is going to have to be either in the GROUP BY clause, be
> functionally dependent on the GROUP BY clause, or be in an aggregate
> function. Putting it into a window function won't help the situation.

Yeah. Also, even if the problem really is that avg(x) should have had
an OVER clause, the fact that the error cursor will not be pointing
at avg(x) means that Vik's wording is still not that helpful.

This is a bit outside our usual error-writing practice, but I wonder
if we could phrase it like "since this query uses aggregation, column
"v.x" must appear in the GROUP BY clause or be used in an aggregate
function". With that, perhaps the user would realize "oh, I didn't
mean to aggregate" when faced with Vik's example. But this phrasing
doesn't cover the GROUP-BY-without-aggregate case, and I'm not sure
how to do that without making the message even longer and more unwieldy.

> If there's any change to make to the error message then it would be to
> add the functional dependency part, but since we're pretty bad at
> detecting that, I don't think we should.

Yeah, that's another thing we're failing to cover in the message ...
but it seems unrelated to Vik's example.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2019-05-24 13:49:30 Re: initdb recommendations
Previous Message David Rowley 2019-05-24 13:33:05 Re: Excessive memory usage in multi-statement queries w/ partitioning