Re: review: More frame options in window functions

From: Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Erik Rijkers <er(at)xs4all(dot)nl>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: review: More frame options in window functions
Date: 2010-01-23 07:43:27
Message-ID: e08cc0401001222343u578972c8o4eb24af38fe0daf3@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2010/1/23 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Tue, Jan 19, 2010 at 3:02 PM, Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com> wrote:
>> 2010/1/19 Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>:
>>> Yeah, that's my point, too. The planner has to distinguish "four" from
>>> sort pathkeys and to teach the executor the simple information which
>>> column should be used to determine frame. I was bit wrong because some
>>> of current executor code isn't like it, like using ordNumCols == 0 to
>>> know whether partition equals to frame, though....
>>
>> And here's another version to fix this problem (I hope). Now the
>> planner distinguish sort column from actual significant pathkeys. I
>> tested it on both of 32bit and 64bit Linux.
>
> Would it make sense to pull some of the infrastructure bits out of
> this patch and commit those bits separately, so as to reduce the size
> of the main patch?  In particular, the AggGetMemoryContext() stuff
> looks like a good candidate for that treatment.

Fair enough. Attached contains that part only. I'll search more parts
like what you suggest, but there seems to be few parts because for
example the change of parser affects all the road to the executor.

> Why did you change BETWEEN from a TYPE_FUNC_NAME_KEYWORD to a COL_NAME_KEYWORD?

Introducing new frame option syntax, shift/reduce conflict came happened.
http://archives.postgresql.org/message-id/e08cc0400911240908s7efaea85wc8505d228220b980@mail.gmail.com
http://archives.postgresql.org/message-id/6363.1229890896@sss.pgh.pa.us

Tom suggested it as RESERVED_KEYWORD a year ago, but COL_NAME_KEYWORD
works as well which is slightly more weak (ie more chances that you
can use the word) than RESERVED_KEYWORD. I'm not completely sure which
is better, though.

Regards,

--
Hitoshi Harada

Attachment Content-Type Size
agg_memorycontext.20100123.patch application/octet-stream 11.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Stark 2010-01-23 11:25:52 Re: 8.5 vs. 9.0, Postgres vs. PostgreSQL
Previous Message KaiGai Kohei 2010-01-23 07:39:17 Re: Largeobject Access Controls (r2460)