On 23 April 2016 at 09:19, Robert Haas <robertmhaas@gmail.com> wrote:
> 2. Add a field "bool aggcombine" to args, and set it to true in this
> case. When we see that in deparsing, expect the argument list to be
> one element long, a TargetEntry containing a Var. Use that to dig out
> the partial Aggref to which it points, and deparse that instead. I
> guess maybe get_variable() could be used for this purpose.
>
I did do that at one stage. I think perhaps one of the above patches
did it that way. The problem was that because I was just detecting
combine Aggrefs and printing the first item in args, it meant the
PARTIAL word was printed again, and that was pretty bogus, since it
was not a partial agg. I didn't see a way to do this without adding
some extra bool parameter to that whole series of functions. But I
only looked at using get_rule_expr(). I didn't look at what
get_variable() is.
> There might be another approach, too. Thoughts?
>
> (Note that I'm assuming here that the final aggregate's target list
> output should match what we would have gotten from a regular
> aggregate, despite the combining stage in the middle. I think that's
> correct; we're outputting the same thing, even though we computed it
> differently.)
Please note that in this case the final and combine node are the same
node, so I'm confused by the "combining stage in the middle" part.
There's only 2 aggregate nodes. I'm not sure how one of those is in
the middle.
I really don't think that we should print FILTER details in a combine
aggregate node. We'd be claiming to be doing something that we're
actually not doing. Please see advance_aggregates() in nodeAgg.c, and
compare that to combine_aggregates(), which is used when combineStates
== true. Notice that only advance_aggregates() bothers with the
aggfilter clause.
-- David Rowley http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
