Re: EXPLAIN VERBOSE with parallel Aggregate

On Sun, Apr 17, 2016 at 10:22 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> writes:
>> On 16 April 2016 at 04:27, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> +1 for the latter, if we can do it conveniently. I think exposing
>>> the names of the aggregate implementation functions would be very
>>> user-unfriendly, as nobody but us hackers knows what those are.
>
>> It does not really seem all that convenient to do this. It also seems
>> a bit strange to me to have a parent node report a column which does
>> not exist in any nodes descending from it. Remember that the combine
>> Aggref does not have the same ->args as its corresponding partial
>> Aggref. It's not all that clear to me if there is any nice way to do
>> have this work the way you'd like. If we were to just call
>> get_rule_expr() on the first arg of the combine aggregate node, it
>> would re-print the PARTIAL keyword again.
>
> This suggests to me that the parsetree representation for partial
> aggregates was badly chosen. If EXPLAIN can't recognize them, then
> neither can anything else, and we may soon find needs for telling
> the difference that are not just cosmetic. Maybe we need more
> fields in Aggref.

So the basic problem is this code in setrefs.c:

newvar = search_indexed_tlist_for_partial_aggref(aggref,
context->subplan_itlist,
context->newvarno);
if (newvar)
{
Aggref *newaggref;
TargetEntry *newtle;

/*
* Now build a new TargetEntry for the Aggref's arguments which is
* a single Var which references the corresponding AggRef in the
* node below.
*/
newtle = makeTargetEntry((Expr *) newvar, 1, NULL, false);
newaggref = (Aggref *) copyObject(aggref);
newaggref->args = list_make1(newtle);

So what ends up happening is that, before setrefs processing, the
FinalizeAggregate's aggref has the same argument list as what the user
originally specified, but during that process, we replace the original
argument list with a 1-element argument list that points to the output
of the partial aggregate step. After that, it's unsurprising that the
deparsing logic goes wrong; consider especially the case of an
aggregate that originally took any number of arguments other than one.
Offhand, I see two somewhat reasonable strategies for trying to fix
this:

1. Add a field "List *origargs" to Aggref, and in this case set
newaggref->origargs to a copy of the original argument list. Use
origargs for deparsing, unless it's NULL, in which case use args. Or
alternative, always populate origargs, but in other cases just make it
equal to args.

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.

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.)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company