add more frame types in window functions (ROWS)
in [PgSql]
|
From: Andrew Gierth on 14 Nov 2009 15:33 Hi, I've started reviewing your patch. I've already found some things that need work: - missing _readWindowFrameDef function (all nodes that are output from parse analysis must have both _read and _out functions, otherwise views can't work) - the A_Const nodes should probably be transformed to Const nodes in parse analysis, since A_Const has no _read/_out functions, which means changing the corresponding code in the executor. (A question here: the spec allows (by my reading) the use of parameters in the window frame clause, i.e. BETWEEN $1 PRECEDING AND $2 FOLLOWING. Wouldn't it therefore make more sense to treat the values as Exprs, albeit very limited ones, and eval them at startup rather than assuming we know the node type and digging down into it all over the place?) You can see the problem here if you do this: create view v as select i,sum(i) over (order by i rows between 1 preceding and 1 following) from generate_series(1,10) i; select * from v; (A regression test for this would probably be good, which reminds me that I need to add one of those myself in the aggregate order by stuff.) Also, you're going to need to do some work in ruleutils.c (get_rule_windowspec) in order to be able to deparse your new frame definitions. I'll continue reviewing the stuff that does work, so I'm not marking this as "waiting for author" yet. -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 14 Nov 2009 15:43 Andrew Gierth <andrew(a)tao11.riddles.org.uk> writes: > (A question here: the spec allows (by my reading) the use of > parameters in the window frame clause, i.e. BETWEEN $1 PRECEDING AND > $2 FOLLOWING. Wouldn't it therefore make more sense to treat the > values as Exprs, albeit very limited ones, and eval them at startup > rather than assuming we know the node type and digging down into it > all over the place?) Seems like you might as well allow any expression not containing local Vars. Compare the handling of LIMIT. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Hitoshi Harada on 15 Nov 2009 02:04 2009/11/15 Tom Lane <tgl(a)sss.pgh.pa.us>: > Andrew Gierth <andrew(a)tao11.riddles.org.uk> writes: >> (A question here: the spec allows (by my reading) the use of >> parameters in the window frame clause, i.e. BETWEEN $1 PRECEDING AND >> $2 FOLLOWING. Wouldn't it therefore make more sense to treat the >> values as Exprs, albeit very limited ones, and eval them at startup >> rather than assuming we know the node type and digging down into it >> all over the place?) > > Seems like you might as well allow any expression not containing > local Vars. Compare the handling of LIMIT. Hmm, I've read it wrong, was assuming a constant for <unsigned value specification> which actually includes any expression. But it's a fixed value during execution, right? Otherwise, we cannot predicate frame boundary. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Hitoshi Harada on 15 Nov 2009 02:11 Thanks for your review. 2009/11/15 Andrew Gierth <andrew(a)tao11.riddles.org.uk>: > Hi, I've started reviewing your patch. > > I've already found some things that need work: > > - missing _readWindowFrameDef function (all nodes that are output > from parse analysis must have both _read and _out functions, > otherwise views can't work) I added _outWindowFramedef() but seem to forget _read one. Will add it. > > - the A_Const nodes should probably be transformed to Const nodes in > parse analysis, since A_Const has no _read/_out functions, which > means changing the corresponding code in the executor. Thanks for this comment. I hadn't determined which node should be used as a value node passed to executor. Including Tom's comment, I must consider which should be again. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Andrew Gierth on 15 Nov 2009 02:33 >>>>> "Hitoshi" == Hitoshi Harada <umi.tanuki(a)gmail.com> writes: >>> (A question here: the spec allows (by my reading) the use of >>> parameters in the window frame clause, i.e. BETWEEN $1 PRECEDING >>> AND $2 FOLLOWING. Wouldn't it therefore make more sense to treat >>> the values as Exprs, albeit very limited ones, and eval them at >>> startup rather than assuming we know the node type and digging >>> down into it all over the place?) >> Seems like you might as well allow any expression not containing >> local Vars. Compare the handling of LIMIT. Hitoshi> Hmm, I've read it wrong, was assuming a constant for <unsigned value Hitoshi> specification> which actually includes any expression. But it's a Hitoshi> fixed value during execution, right? Otherwise, we cannot predicate Hitoshi> frame boundary. The spec doesn't allow arbitrary expressions there, only literals and parameters. Allowing more than that would be going beyond the spec, but as with LIMIT, could be useful nonetheless. For it to be a fixed value during execution, the same rules apply as for LIMIT; it can't contain Vars of the current query level. My thinking is that the executor definitely shouldn't be relying on it being a specific node type, but should just ExecEvalExpr it on the first call and store the result; then you don't have to know whether it's a Const or Param node or a more complex expression. -- Andrew. -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
|
Next
|
Last
Pages: 1 2 3 4 Prev: patch - per-tablespace random_page_cost/seq_page_cost Next: named parameters in SQL functions |