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 > sort > >> > 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. Regards, Hari Babu Fujitsu Australia
