Re: review: More frame options in window functions

2010/1/17 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com> writes:
>> ... I tried to change not to canonicalize the
>> pathkeys in make_pathkeys_window() in such cases and succeeded then
>> passed all regression tests.
>
> That's broken, whether it passes regression tests or not.  Not
> canonicalizing will mean that you fail to recognize equality to
> canonicalized pathkeys, and thus for example execute unnecessary
> sorts.

So why did you leave "canonicalize" argument in
make_pathkeys_for_window()? I thought you'd thought it would be needed
false in the future.

> I haven't looked at the patch, but it sounds a bit like you are trying
> to put logic into the executor that needs to be in the planner.  If the
> executor is guessing about what the planner did, that's a design
> failure.  The planner should figure out what needs to happen and tell
> the executor exactly what to do, eg, which columns need to be compared
> to determine partition membership.

I'd think I'm putting logic into the planner that needs to be in the
executor. Anyways, RANGE offset mode is quite different from existing
framework.

In a RANGE offset mode query, for example:

SELECT sum(ten) over (PARTITION BY four ORDER BY four RANGE BETWEEN 2
PRECEDING AND 1 PRECEDING) FROM tenk1

the frame is determined as "from the first row which has <four> value
- 2 to the last row which has <four> value - 1" and executor should
know <four> value *is* the sort column even if the column is not
actually significant. But the planner removes that information. It
seems to me that what you say above is to put logic into the planner
how to determine the frame, but I'd think the planner should leave
that information about sort column for the executor, of course in
another way than non-canonicalizing approach.

I'll try it that way. If I'm missing something still, please point it out.

Regards,

--
Hitoshi Harada