Re: Document aggregate functions better w.r.t. ORDER BY

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Document aggregate functions better w.r.t. ORDER BY
Date: 2023-10-26 02:12:56
Message-ID: CAApHDvpmQgWo8gqFmZ7Ot1cRazDiAKDBmQNJZTt7Waifo7htSw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 26 Oct 2023 at 13:10, David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
> Question: Do you know whether we for certain always sort ascending here to compute the unique values or whether if, say, there is an index on the column in descending order (or ascending and traversed backwards) that the data within the aggregate could, with an order by, be returned in descending order?

The way it's currently coded, we seem to always require ascending
order. See addTargetToGroupList(). The call to
get_sort_group_operators() only requests the ltOpr.

A quick test creating an index on a column with DESC shows that we end
up doing a backwards index scan so that we get the requested ascending
order:

create table b (b text);
create index on b (b desc);
explain select string_agg(distinct b,',') from b;
QUERY PLAN
------------------------------------------------------------------------------------------
Aggregate (cost=67.95..67.97 rows=1 width=32)
-> Index Only Scan Backward using b_b_idx on b (cost=0.15..64.55
rows=1360 width=32)
(2 rows)

However, I think we'd best stay clear of offering any guarantees in
the documents about this. If we did that it would be much harder in
the future if we wanted to implement the DISTINCT aggregates by
hashing.

David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2023-10-26 02:32:13 Re: remaining sql/json patches
Previous Message Michael Paquier 2023-10-26 01:59:58 Re: Introduce a new view for checkpointer related stats