On Fri, Dec 2, 2016 at 4:05 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Tue, Sep 13, 2016 at 4:32 AM, Alexander Korotkov
> <aekorot...@gmail.com> wrote:
> > On Fri, Apr 8, 2016 at 10:09 PM, Peter Geoghegan <p...@heroku.com> wrote:
> >> On Wed, Mar 30, 2016 at 8:02 AM, Alexander Korotkov
> >> <aekorot...@gmail.com> wrote:
> >> > Hmm... I'm not completely agree with that. In typical usage partial
> >> > should definitely use quicksort. However, fallback to other sort
> >> > methods is
> >> > very useful. Decision of partial sort usage is made by planner. But
> >> > planner makes mistakes. For example, our HashAggregate is purely
> >> > in-memory.
> >> > In the case of planner mistake it causes OOM. I met such situation in
> >> > production and not once. This is why I'd like partial sort to have
> >> > graceful
> >> > degradation for such cases.
> >> I think that this should be moved to the next CF, unless a committer
> >> wants to pick it up today.
> > Patch was rebased to current master.
> Just a few quick observations on this...
> It strikes me that the API contract change in ExecMaterializesOutput
> is pretty undesirable. I think it would be better to have a new
> executor node for this node rather than overloading the existing
> "Sort" node, sharing code where possible of course. The fact that
> this would distinguish them more clearly in an EXPLAIN plan seems
> good, too. "Partial Sort" is the obvious thing, but there might be
> even better alternatives -- maybe "Incremental Sort" or something like
> that? Because it's not partially sorting the data, it's making data
> that already has some sort order have a more rigorous sort order.
> I think that it will probably be pretty common to have queries where
> the data is sorted by (mostly_unique_col) and we want to get it sorted
> by (mostly_unique_col, disambiguation_col). In such cases I wonder if
> we'll incur a lot of overhead by feeding single tuples to the
> tuplesort stuff and performing lots of 1-item sorts. Not sure if that
> case needs any special optimization.
> I also think that the "HeapTuple prev" bit in SortState is probably
> not the right way of doing things. I think that should use an
> additional TupleTableSlot rather than a HeapTuple.
The feedback from the reviewer has received at the end of commitfest.
So Moved to next CF with "waiting on author" status.