Re: Window functions patch v04 for the September commit fest

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: Window functions patch v04 for the September commit fest
Date: 2008-09-04 18:11:13
Message-ID: 48C024C1.3070107@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kenneth Marshall 2008-09-04 18:48:17 Re: Need more reviewers!
Previous Message Tom Lane 2008-09-04 18:01:18 Re: Need more reviewers!