Re: Window functions patch v04 for the September commit fest - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: Window functions patch v04 for the September commit fest
Date
Msg-id 48C024C1.3070107@enterprisedb.com
Whole thread Raw
In response to Re: Window functions patch v04 for the September commit fest  (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>)
Responses Re: Window functions patch v04 for the September commit fest  ("Hitoshi Harada" <umi.tanuki@gmail.com>)
List pgsql-hackers
Heikki Linnakangas wrote:
> I'll review the parser/planner changes from the current patch.

Looks pretty sane to me. Few issues:

Is it always OK to share a window between two separate window function 
invocations, if they both happen to have identical OVER clause? It seems 
OK for stable functions, but I'm not sure that's correct for expressions 
involving volatile functions. I wonder if the SQL spec has anything to 
say about that.

As you noted in the comments, the planner could be a lot smarter about 
avoiding sorts. Currently you just always put a sort node below each 
Window, and another on top of them if there's an ORDER BY. There's 
obviously a lot of room for improvement there.

This line:
>         result_plan->targetlist = preprocess_window(tlist, result_plan);
in planner.c, won't work if result_plan is one that can't do projection.  A few screenfuls above that call, there's
this:
>                 /*
>                  * If the top-level plan node is one that cannot do expression
>                  * evaluation, we must insert a Result node to project the
>                  * desired tlist.
>                  */
>                 if (!is_projection_capable_plan(result_plan))
>                 {
>                     result_plan = (Plan *) make_result(root,
>                                                        sub_tlist,
>                                                        NULL,
>                                                        result_plan);
>                 }
>                 else
>                 {
>                     /*
>                      * Otherwise, just replace the subplan's flat tlist with
>                      * the desired tlist.
>                      */
>                     result_plan->targetlist = sub_tlist;
>                 }
which is what you need to do with the preprocess_window call as well. I 
think this is caused by that:
postgres=# explain SELECT row_number() OVER (ORDER BY id*10) FROM 
(SELECT * FROM foo UNION ALL SELECT * FROM foo) sq;
ERROR:  bogus varattno for OUTER var: 2

And then there's these:

postgres=# explain SELECT * FROm foo WHERE row_number() OVER (ORDER BY 
id) > 10;
ERROR:  winaggref found in non-Window plan node
postgres=# explain UPDATE foo SET id = 10 RETURNING (ROW_NUMBER() OVER 
(ORDER BY random())) ;
ERROR:  winaggref found in non-Window plan node
postgres=# explain SELECT row_number() OVER (ORDER BY (1)) FROm foo;
ERROR:  ORDER/GROUP BY expression not found in targetlist
postgres=# explain SELECT row_number() OVER (ORDER BY length('foo')) 
FROM foo;
ERROR:  could not find pathkey item to sort

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Need more reviewers!
Next
From: Kenneth Marshall
Date:
Subject: Re: Need more reviewers!