Re: Windowing Function Patch Review -> Standard Conformance

Hitoshi Harada Wrote:
> 2008/12/3 Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>:
> > I am randomly trying some issues instead of agg common code (which I
> > now doubt if it's worth sharing the code), so tell me if you're
> > restarting your hack again. I'll send the whole patch.
> >
>
> Attached is the updated patch, including:
>
> - performance tuning up for large data sets
> - move find_wfunc to optimizer/util/clauses.c
> - rename WFunc to WindowFunc
> - rename WinDef to WindowDef
> - rename wfunc->pure_agg to winagg
> - remove winstate->tail_ptr
> - fix WinFrameGetArg in case that there are more than one peer, add
> relevant test
> - duplicate GetAggInitVal(), not sharing aggregate routines with nodeAgg.c
>
> I believe the remaining work is only to optimze row_number()/rank()
> cases to trim tuplestore like window aggregates. This will be done by
> providing some kind of way for each window functions to tell Window
> node that it doesn't require backward_scan.

It's been a long time since the CommitFest started. This patch has come a
long way in that time. We've seen various performance improvements and many
fixes where the patch was not matching the standard. We've also seen quite a
big change in the window buffering technique which is showing amazing
performance improvements in certain queries.

I've spent many hours reading the standard and comparing the results from
this patch against other RDBMS' that support window functions. I wonder if
we're the first to implement NTH_VALUE()?.

The patch, in my opinion is getting very close to being committable. The
only outstanding issues; Please correct me where I'm wrong or have omitted
something.

* Fix still required for rank_up. Code still has a FIXME.

* ROW_NUMBER() and RANK() performance to be looked into.

* Slight changes required in documentation (my previous email).

* Final code read by someone reviewing the code.

I've also spent many hours trying to break this patch and in previous
versions I've managed it. Last night I spent a few hours again testing this
new patch and did not managed to find anything wrong. We're getting close to

the time where the community can test further by committing this patch.

I'll make a reference to this post in the CommitFest list for any
non-followers of this thread.

David.