Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 59eb551

Browse files
committed
Fix EXPLAIN VERBOSE output for parallel aggregate.
The way that PartialAggregate and FinalizeAggregate plan nodes were displaying output columns before was bogus. Now, FinalizeAggregate produces the same outputs as an Aggregate would have produced, while PartialAggregate produces each of those outputs prefixed by the word PARTIAL. Discussion: 12585.1460737650@sss.pgh.pa.us Patch by me, reviewed by David Rowley.
1 parent 72a98a6 commit 59eb551

File tree

8 files changed

+168
-78
lines changed

8 files changed

+168
-78
lines changed

src/backend/nodes/copyfuncs.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,6 +1244,8 @@ _copyAggref(const Aggref *from)
12441244
COPY_NODE_FIELD(aggfilter);
12451245
COPY_SCALAR_FIELD(aggstar);
12461246
COPY_SCALAR_FIELD(aggvariadic);
1247+
COPY_SCALAR_FIELD(aggpartial);
1248+
COPY_SCALAR_FIELD(aggcombine);
12471249
COPY_SCALAR_FIELD(aggkind);
12481250
COPY_SCALAR_FIELD(agglevelsup);
12491251
COPY_LOCATION_FIELD(location);

src/backend/nodes/equalfuncs.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ _equalAggref(const Aggref *a, const Aggref *b)
202202
COMPARE_NODE_FIELD(aggfilter);
203203
COMPARE_SCALAR_FIELD(aggstar);
204204
COMPARE_SCALAR_FIELD(aggvariadic);
205+
COMPARE_SCALAR_FIELD(aggcombine);
206+
COMPARE_SCALAR_FIELD(aggpartial);
205207
COMPARE_SCALAR_FIELD(aggkind);
206208
COMPARE_SCALAR_FIELD(agglevelsup);
207209
COMPARE_LOCATION_FIELD(location);

src/backend/nodes/outfuncs.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,6 +1040,8 @@ _outAggref(StringInfo str, const Aggref *node)
10401040
WRITE_NODE_FIELD(aggfilter);
10411041
WRITE_BOOL_FIELD(aggstar);
10421042
WRITE_BOOL_FIELD(aggvariadic);
1043+
WRITE_BOOL_FIELD(aggcombine);
1044+
WRITE_BOOL_FIELD(aggpartial);
10431045
WRITE_CHAR_FIELD(aggkind);
10441046
WRITE_UINT_FIELD(agglevelsup);
10451047
WRITE_LOCATION_FIELD(location);

src/backend/nodes/readfuncs.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,8 @@ _readAggref(void)
556556
READ_NODE_FIELD(aggfilter);
557557
READ_BOOL_FIELD(aggstar);
558558
READ_BOOL_FIELD(aggvariadic);
559+
READ_BOOL_FIELD(aggcombine);
560+
READ_BOOL_FIELD(aggpartial);
559561
READ_CHAR_FIELD(aggkind);
560562
READ_UINT_FIELD(agglevelsup);
561563
READ_LOCATION_FIELD(location);

src/backend/optimizer/plan/setrefs.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2100,6 +2100,10 @@ search_indexed_tlist_for_partial_aggref(Aggref *aggref, indexed_tlist *itlist,
21002100
continue;
21012101
if (aggref->aggvariadic != tlistaggref->aggvariadic)
21022102
continue;
2103+
/*
2104+
* it would be harmless to compare aggcombine and aggpartial, but
2105+
* it's also unnecessary
2106+
*/
21032107
if (aggref->aggkind != tlistaggref->aggkind)
21042108
continue;
21052109
if (aggref->agglevelsup != tlistaggref->agglevelsup)
@@ -2463,6 +2467,7 @@ fix_combine_agg_expr_mutator(Node *node, fix_upper_expr_context *context)
24632467
newtle = makeTargetEntry((Expr *) newvar, 1, NULL, false);
24642468
newaggref = (Aggref *) copyObject(aggref);
24652469
newaggref->args = list_make1(newtle);
2470+
newaggref->aggcombine = true;
24662471

24672472
return (Node *) newaggref;
24682473
}

src/backend/optimizer/util/tlist.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,9 @@ apply_partialaggref_adjustment(PathTarget *target)
792792
else
793793
newaggref->aggoutputtype = aggform->aggtranstype;
794794

795+
/* flag it as partial */
796+
newaggref->aggpartial = true;
797+
795798
lfirst(lc) = newaggref;
796799

797800
ReleaseSysCache(aggTuple);

src/backend/utils/adt/ruleutils.c

Lines changed: 150 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,11 @@ static void get_rule_windowspec(WindowClause *wc, List *targetList,
392392
deparse_context *context);
393393
static char *get_variable(Var *var, int levelsup, bool istoplevel,
394394
deparse_context *context);
395+
static void get_special_variable(Node *node, deparse_context *context,
396+
void *private);
397+
static void resolve_special_varno(Node *node, deparse_context *context,
398+
void *private,
399+
void (*callback) (Node *, deparse_context *, void *));
395400
static Node *find_param_referent(Param *param, deparse_context *context,
396401
deparse_namespace **dpns_p, ListCell **ancestor_cell_p);
397402
static void get_parameter(Param *param, deparse_context *context);
@@ -407,7 +412,10 @@ static void get_rule_expr_toplevel(Node *node, deparse_context *context,
407412
static void get_oper_expr(OpExpr *expr, deparse_context *context);
408413
static void get_func_expr(FuncExpr *expr, deparse_context *context,
409414
bool showimplicit);
410-
static void get_agg_expr(Aggref *aggref, deparse_context *context);
415+
static void get_agg_expr(Aggref *aggref, deparse_context *context,
416+
Aggref *original_aggref);
417+
static void get_agg_combine_expr(Node *node, deparse_context *context,
418+
void *private);
411419
static void get_windowfunc_expr(WindowFunc *wfunc, deparse_context *context);
412420
static void get_coercion_expr(Node *arg, deparse_context *context,
413421
Oid resulttype, int32 resulttypmod,
@@ -5877,7 +5885,6 @@ get_utility_query_def(Query *query, deparse_context *context)
58775885
}
58785886
}
58795887

5880-
58815888
/*
58825889
* Display a Var appropriately.
58835890
*
@@ -5930,82 +5937,11 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
59305937
colinfo = deparse_columns_fetch(var->varno, dpns);
59315938
attnum = var->varattno;
59325939
}
5933-
else if (var->varno == OUTER_VAR && dpns->outer_tlist)
5934-
{
5935-
TargetEntry *tle;
5936-
deparse_namespace save_dpns;
5937-
5938-
tle = get_tle_by_resno(dpns->outer_tlist, var->varattno);
5939-
if (!tle)
5940-
elog(ERROR, "bogus varattno for OUTER_VAR var: %d", var->varattno);
5941-
5942-
Assert(netlevelsup == 0);
5943-
push_child_plan(dpns, dpns->outer_planstate, &save_dpns);
5944-
5945-
/*
5946-
* Force parentheses because our caller probably assumed a Var is a
5947-
* simple expression.
5948-
*/
5949-
if (!IsA(tle->expr, Var))
5950-
appendStringInfoChar(buf, '(');
5951-
get_rule_expr((Node *) tle->expr, context, true);
5952-
if (!IsA(tle->expr, Var))
5953-
appendStringInfoChar(buf, ')');
5954-
5955-
pop_child_plan(dpns, &save_dpns);
5956-
return NULL;
5957-
}
5958-
else if (var->varno == INNER_VAR && dpns->inner_tlist)
5959-
{
5960-
TargetEntry *tle;
5961-
deparse_namespace save_dpns;
5962-
5963-
tle = get_tle_by_resno(dpns->inner_tlist, var->varattno);
5964-
if (!tle)
5965-
elog(ERROR, "bogus varattno for INNER_VAR var: %d", var->varattno);
5966-
5967-
Assert(netlevelsup == 0);
5968-
push_child_plan(dpns, dpns->inner_planstate, &save_dpns);
5969-
5970-
/*
5971-
* Force parentheses because our caller probably assumed a Var is a
5972-
* simple expression.
5973-
*/
5974-
if (!IsA(tle->expr, Var))
5975-
appendStringInfoChar(buf, '(');
5976-
get_rule_expr((Node *) tle->expr, context, true);
5977-
if (!IsA(tle->expr, Var))
5978-
appendStringInfoChar(buf, ')');
5979-
5980-
pop_child_plan(dpns, &save_dpns);
5981-
return NULL;
5982-
}
5983-
else if (var->varno == INDEX_VAR && dpns->index_tlist)
5984-
{
5985-
TargetEntry *tle;
5986-
5987-
tle = get_tle_by_resno(dpns->index_tlist, var->varattno);
5988-
if (!tle)
5989-
elog(ERROR, "bogus varattno for INDEX_VAR var: %d", var->varattno);
5990-
5991-
Assert(netlevelsup == 0);
5992-
5993-
/*
5994-
* Force parentheses because our caller probably assumed a Var is a
5995-
* simple expression.
5996-
*/
5997-
if (!IsA(tle->expr, Var))
5998-
appendStringInfoChar(buf, '(');
5999-
get_rule_expr((Node *) tle->expr, context, true);
6000-
if (!IsA(tle->expr, Var))
6001-
appendStringInfoChar(buf, ')');
6002-
6003-
return NULL;
6004-
}
60055940
else
60065941
{
6007-
elog(ERROR, "bogus varno: %d", var->varno);
6008-
return NULL; /* keep compiler quiet */
5942+
resolve_special_varno((Node *) var, context, NULL,
5943+
get_special_variable);
5944+
return NULL;
60095945
}
60105946

60115947
/*
@@ -6118,6 +6054,101 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
61186054
return attname;
61196055
}
61206056

6057+
/*
6058+
* Deparse a Var which references OUTER_VAR, INNER_VAR, or INDEX_VAR. This
6059+
* routine is actually a callback for get_special_varno, which handles finding
6060+
* the correct TargetEntry. We get the expression contained in that
6061+
* TargetEntry and just need to deparse it, a job we can throw back on
6062+
* get_rule_expr.
6063+
*/
6064+
static void
6065+
get_special_variable(Node *node, deparse_context *context, void *private)
6066+
{
6067+
StringInfo buf = context->buf;
6068+
6069+
/*
6070+
* Force parentheses because our caller probably assumed a Var is a simple
6071+
* expression.
6072+
*/
6073+
if (!IsA(node, Var))
6074+
appendStringInfoChar(buf, '(');
6075+
get_rule_expr(node, context, true);
6076+
if (!IsA(node, Var))
6077+
appendStringInfoChar(buf, ')');
6078+
}
6079+
6080+
/*
6081+
* Chase through plan references to special varnos (OUTER_VAR, INNER_VAR,
6082+
* INDEX_VAR) until we find a real Var or some kind of non-Var node; then,
6083+
* invoke the callback provided.
6084+
*/
6085+
static void
6086+
resolve_special_varno(Node *node, deparse_context *context, void *private,
6087+
void (*callback) (Node *, deparse_context *, void *))
6088+
{
6089+
Var *var;
6090+
deparse_namespace *dpns;
6091+
6092+
/* If it's not a Var, invoke the callback. */
6093+
if (!IsA(node, Var))
6094+
{
6095+
callback(node, context, private);
6096+
return;
6097+
}
6098+
6099+
/* Find appropriate nesting depth */
6100+
var = (Var *) node;
6101+
dpns = (deparse_namespace *) list_nth(context->namespaces,
6102+
var->varlevelsup);
6103+
6104+
/*
6105+
* It's a special RTE, so recurse.
6106+
*/
6107+
if (var->varno == OUTER_VAR && dpns->outer_tlist)
6108+
{
6109+
TargetEntry *tle;
6110+
deparse_namespace save_dpns;
6111+
6112+
tle = get_tle_by_resno(dpns->outer_tlist, var->varattno);
6113+
if (!tle)
6114+
elog(ERROR, "bogus varattno for OUTER_VAR var: %d", var->varattno);
6115+
6116+
push_child_plan(dpns, dpns->outer_planstate, &save_dpns);
6117+
resolve_special_varno((Node *) tle->expr, context, private, callback);
6118+
pop_child_plan(dpns, &save_dpns);
6119+
return;
6120+
}
6121+
else if (var->varno == INNER_VAR && dpns->inner_tlist)
6122+
{
6123+
TargetEntry *tle;
6124+
deparse_namespace save_dpns;
6125+
6126+
tle = get_tle_by_resno(dpns->inner_tlist, var->varattno);
6127+
if (!tle)
6128+
elog(ERROR, "bogus varattno for INNER_VAR var: %d", var->varattno);
6129+
6130+
push_child_plan(dpns, dpns->inner_planstate, &save_dpns);
6131+
resolve_special_varno((Node *) tle->expr, context, private, callback);
6132+
pop_child_plan(dpns, &save_dpns);
6133+
return;
6134+
}
6135+
else if (var->varno == INDEX_VAR && dpns->index_tlist)
6136+
{
6137+
TargetEntry *tle;
6138+
6139+
tle = get_tle_by_resno(dpns->index_tlist, var->varattno);
6140+
if (!tle)
6141+
elog(ERROR, "bogus varattno for INDEX_VAR var: %d", var->varattno);
6142+
6143+
resolve_special_varno((Node *) tle->expr, context, private, callback);
6144+
return;
6145+
}
6146+
else if (var->varno < 1 || var->varno > list_length(dpns->rtable))
6147+
elog(ERROR, "bogus varno: %d", var->varno);
6148+
6149+
/* Not special. Just invoke the callback. */
6150+
callback(node, context, private);
6151+
}
61216152

61226153
/*
61236154
* Get the name of a field of an expression of composite type. The
@@ -7080,7 +7111,7 @@ get_rule_expr(Node *node, deparse_context *context,
70807111
break;
70817112

70827113
case T_Aggref:
7083-
get_agg_expr((Aggref *) node, context);
7114+
get_agg_expr((Aggref *) node, context, (Aggref *) node);
70847115
break;
70857116

70867117
case T_GroupingFunc:
@@ -8236,13 +8267,36 @@ get_func_expr(FuncExpr *expr, deparse_context *context,
82368267
* get_agg_expr - Parse back an Aggref node
82378268
*/
82388269
static void
8239-
get_agg_expr(Aggref *aggref, deparse_context *context)
8270+
get_agg_expr(Aggref *aggref, deparse_context *context,
8271+
Aggref *original_aggref)
82408272
{
82418273
StringInfo buf = context->buf;
82428274
Oid argtypes[FUNC_MAX_ARGS];
82438275
int nargs;
82448276
bool use_variadic;
82458277

8278+
/*
8279+
* For a combining aggregate, we look up and deparse the corresponding
8280+
* partial aggregate instead. This is necessary because our input
8281+
* argument list has been replaced; the new argument list always has just
8282+
* one element, which will point to a partial Aggref that supplies us with
8283+
* transition states to combine.
8284+
*/
8285+
if (aggref->aggcombine)
8286+
{
8287+
TargetEntry *tle = linitial(aggref->args);
8288+
8289+
Assert(list_length(aggref->args) == 1);
8290+
Assert(IsA(tle, TargetEntry));
8291+
resolve_special_varno((Node *) tle->expr, context, original_aggref,
8292+
get_agg_combine_expr);
8293+
return;
8294+
}
8295+
8296+
/* Mark as PARTIAL, if appropriate. */
8297+
if (original_aggref->aggpartial)
8298+
appendStringInfoString(buf, "PARTIAL ");
8299+
82468300
/* Extract the argument types as seen by the parser */
82478301
nargs = get_aggregate_argtypes(aggref, argtypes);
82488302

@@ -8311,6 +8365,24 @@ get_agg_expr(Aggref *aggref, deparse_context *context)
83118365
appendStringInfoChar(buf, ')');
83128366
}
83138367

8368+
/*
8369+
* This is a helper function for get_agg_expr(). It's used when we deparse
8370+
* a combining Aggref; resolve_special_varno locates the corresponding partial
8371+
* Aggref and then calls this.
8372+
*/
8373+
static void
8374+
get_agg_combine_expr(Node *node, deparse_context *context, void *private)
8375+
{
8376+
Aggref *aggref;
8377+
Aggref *original_aggref = private;
8378+
8379+
if (!IsA(node, Aggref))
8380+
elog(ERROR, "combining Aggref does not point to an Aggref");
8381+
8382+
aggref = (Aggref *) node;
8383+
get_agg_expr(aggref, context, original_aggref);
8384+
}
8385+
83148386
/*
83158387
* get_windowfunc_expr - Parse back a WindowFunc node
83168388
*/

src/include/nodes/primnodes.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,8 @@ typedef struct Aggref
280280
bool aggstar; /* TRUE if argument list was really '*' */
281281
bool aggvariadic; /* true if variadic arguments have been
282282
* combined into an array last argument */
283+
bool aggcombine; /* combining agg; input is a transvalue */
284+
bool aggpartial; /* partial agg; output is a transvalue */
283285
char aggkind; /* aggregate kind (see pg_aggregate.h) */
284286
Index agglevelsup; /* > 0 if agg belongs to outer query */
285287
int location; /* token location, or -1 if unknown */

0 commit comments

Comments
 (0)