On Wed, Oct 12, 2022 at 5:35 PM David Rowley <dgrowle...@gmail.com> wrote:
> On Wed, 12 Oct 2022 at 16:33, Vik Fearing <v...@postgresfriends.org> wrote: > > Per spec, the ROW_NUMBER() window function is not even allowed to have a > > frame specified. > > > > b) The window framing clause of WDX shall not be present. > > > > Also, the specification for ROW_NUMBER() is: > > > > f) ROW_NUMBER() OVER WNS is equivalent to the <window function>: > > > > COUNT (*) OVER (WNS1 ROWS UNBOUNDED PRECEDING) > > > > > > So I don't think we need to test for anything at all and can > > indiscriminately add or replace the frame with ROWS UNBOUNDED PRECEDING. > > Thanks for digging that out. > > Just above that I see: > > RANK() OVER WNS is equivalent to: > ( COUNT (*) OVER (WNS1 RANGE UNBOUNDED PRECEDING) > - COUNT (*) OVER (WNS1 RANGE CURRENT ROW) + 1 ) > > and > > DENSE_RANK() OVER WNS is equivalent to the <window function>: > COUNT (DISTINCT ROW ( VE1, ..., VEN ) ) > OVER (WNS1 RANGE UNBOUNDED PRECEDING) > > So it looks like the same can be done for rank() and dense_rank() too. > I've added support for those in the attached. > > This also got me thinking that maybe we should be a bit more generic > with the support function node tag name. After looking at the > nodeWindowAgg.c code for a while, I wondered if we might want to add > some optimisations in the future that makes WindowAgg not bother > storing tuples for row_number(), rank() and dense_rank(). That might > save a bit of overhead from the tuple store. I imagined that we'd > want to allow the expansion of this support request so that the > support function could let the planner know if any tuples will be > accessed by the window function or not. The > SupportRequestWFuncOptimizeFrameOpts name didn't seem very fitting for > that so I adjusted it to become SupportRequestOptimizeWindowClause > instead. > > The updated patch is attached. > > David > Hi, + req->frameOptions = (FRAMEOPTION_ROWS | + FRAMEOPTION_START_UNBOUNDED_PRECEDING | + FRAMEOPTION_END_CURRENT_ROW); The bit combination appears multiple times in the patch. Maybe define the combination as a constant in supportnodes.h and reference it in the code. Cheers
