Re: Add proper planner support for ORDER BY / DISTINCT aggregates

From: Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io>
To: David Rowley <dgrowleyml(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Pavel Luzanov <p(dot)luzanov(at)postgrespro(dot)ru>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Richard Guo <guofenglinux(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Date: 2022-11-07 17:30:16
Message-ID: 3699971.kQq0lBPeGt@aivenlaptop
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Le lundi 7 novembre 2022, 17:58:50 CET Pavel Luzanov a écrit :
> > I can't see why an incrementalsort could be more expensive than a full
> > sort, using the same presorted path.
>
> The only reason I can see is the number of buffers to read. In the plan
> with incremental sort we read the whole index, ~190000 buffers.
> And the plan with seq scan only required ~5000 (I think due to buffer
> ring optimization).

What I meant here is that disabling seqscans, the planner still chooses a full
sort over a partial sort. The underlying index is the same, it is just a
matter of choosing a Sort node over an IncrementalSort node. This, I think, is
wrong: I can't see how it could be worse to use an incrementalsort in that
case.

It makes sense to prefer a SeqScan over an IndexScan if you are going to sort
the whole table anyway. But in that case we shouldn't. What happened before is
that some sort of incremental sort was always chosen, because it was hidden as
an implementation detail of the agg node. But now it has to compete on a cost
basis with the full sort, and that costing is wrong in that case.

Maybe the original costing code for incremental sort was a bit too
pessimistic.

--
Ronan Dunklau

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-11-07 17:42:45 Re: hash_xlog_split_allocate_page: failed to acquire cleanup lock
Previous Message Andres Freund 2022-11-07 17:13:55 allow segment size to be set to < 1GiB