Hitoshi == Hitoshi Harada umi.tan...@gmail.com writes:
Hitoshi Attached is updated version. I added AggGetMemoryContext()
Hitoshi in executor/nodeAgg.h (though I'm not sure where to go...)
Hitoshi and its second argument iswindowagg is output parameter to
Hitoshi know whether the call
2009/12/7 Andrew Gierth and...@tao11.riddles.org.uk:
Hitoshi == Hitoshi Harada umi.tan...@gmail.com writes:
Hitoshi Attached is updated version. I added AggGetMemoryContext()
Hitoshi in executor/nodeAgg.h (though I'm not sure where to go...)
Hitoshi and its second argument iswindowagg is
Hitoshi == Hitoshi Harada umi.tan...@gmail.com writes:
So for this and the regression test problem mentioned in the other
mail, I'm setting this back to waiting on author.
Hitoshi In my humble opinion, view regression test is not necessary
Hitoshi in both my patch and yours. At least
Andrew Gierth wrote:
But what you have in the regression tests _now_ is, as far as I can
tell, only working by chance; with rules and window being in the
same parallel group, and window.sql creating a view, you have an
obvious race condition, unless I'm missing something.
You're right.
2009/12/7 Greg Smith g...@2ndquadrant.com:
Which means that views created in the window test could absolutely cause the
rules test to fail given a bad race condition. Either rules or window needs
to be moved to another section of the test schedule. (I guess you could cut
down the scope of
2009/12/5 Andrew Gierth and...@tao11.riddles.org.uk:
1) Memory context handling for aggregate calls
aggcontext = AggGetMemoryContext(fcinfo-context);
if (!aggcontext)
ereport(...);
For completeness, there should be two other functions: one to simply
return whether we are in
2009/12/5 Hitoshi Harada umi.tan...@gmail.com:
I'm now reworking as reviewed. The last issue is whether we accept
extension of frame types without RANGE support.
Attached is updated version. I added AggGetMemoryContext() in
executor/nodeAgg.h (though I'm not sure where to go...) and its second
Hitoshi == Hitoshi Harada umi.tan...@gmail.com writes:
Hitoshi One thing for rule test, I checked existing regression test
Hitoshi cases and concluded DROP VIEW is necessary, or even VIEW
Hitoshi test for a specific feature is not needed. I remember your
Hitoshi aggregate ORDER BY patch
Functionally this patch looks excellent; correct format, applies
cleanly, passes regression, and I've been unable to find any issues
with the code itself. But I still have a concern over the interface
change, so I'm setting this back to waiting on author for now even
though it's really a matter
Andrew Gierth and...@tao11.riddles.org.uk writes:
If we're going to change the interface in this way, there should, IMO,
be enough of a change that old code fails to compile; e.g. by renaming
wincontext to partition_context or some equivalent change.
Agreed --- if we have to break things,
Hitoshi == Hitoshi Harada umi.tan...@gmail.com writes:
Hitoshi As earlier mail, I added aggcontext to WindowAggState.
This issue (as detailed in this post):
http://archives.postgresql.org/pgsql-hackers/2009-11/msg01871.php
is currently the only significant outstanding issue in my review of
Here's the rest of the review, as far as I've taken it given the
problems with the code.
The patch applied cleanly and includes regression tests but not docs.
Small nitpicks: there are some comments not updated (e.g. the
big one at the start of eval_windowaggregates). A couple of lines are
2009/11/19 Andrew Gierth and...@tao11.riddles.org.uk:
Here's the rest of the review, as far as I've taken it given the
problems with the code.
The patch applied cleanly and includes regression tests but not docs.
Small nitpicks: there are some comments not updated (e.g. the
big one at the
2009/11/15 Tom Lane t...@sss.pgh.pa.us:
Andrew Gierth and...@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
Thanks for your review.
2009/11/15 Andrew Gierth and...@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
Hitoshi == Hitoshi Harada umi.tan...@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
2009/11/15 Andrew Gierth and...@tao11.riddles.org.uk:
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
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
Andrew Gierth and...@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
19 matches
Mail list logo
