Skip to content

Commit 11e1318

Browse files
tglsfdctglsfdc
committed
Improve ruleutils.c's heuristics for dealing with rangetable aliases.
The previous scheme had bugs in some corner cases involving tables that had been renamed since a view was made. This could result in dumped views that failed to reload or reloaded incorrectly, as seen in bug #7553 from Lloyd Albin, as well as in some pgsql-hackers discussion back in January. Also, its behavior for printing EXPLAIN plans was sometimes confusing because of willingness to use the same alias for multiple RTEs (it was Ashutosh Bapat's complaint about that aspect that started the January thread). To fix, ensure that each RTE in the query has a unique unqualified alias, by modifying the alias if necessary (we add "_" and digits as needed to create a non-conflicting name). Then we can just print its variables with that alias, avoiding the confusing and bug-prone scheme of sometimes schema-qualifying variable names. In EXPLAIN, it proves to be expedient to take the further step of only assigning such aliases to RTEs that are actually referenced in the query, since the planner has a habit of generating extra RTEs with the same alias in situations such as inheritance-tree expansion. Although this fixes a bug of very long standing, I'm hesitant to back-patch such a noticeable behavioral change. My experiments while creating a regression test convinced me that actually incorrect output (as opposed to confusing output) occurs only in very narrow cases, which is backed up by the lack of previous complaints from the field. So we may be better off living with it in released branches; and in any case it'd be smart to let this ripen awhile in HEAD before we consider back-patching it.
1 parent 7c45e3a commit 11e1318

File tree

12 files changed

+886
-226
lines changed

12 files changed

+886
-226
lines changed

‎src/backend/commands/explain.c

+147-8
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ static void ExplainOneQuery(Query *query, IntoClause *into, ExplainState *es,
5151
static void report_triggers(ResultRelInfo *rInfo, bool show_relname,
5252
ExplainState *es);
5353
static double elapsed_time(instr_time *starttime);
54+
static void ExplainPreScanNode(PlanState *planstate, Bitmapset **rels_used);
55+
static void ExplainPreScanMemberNodes(List *plans, PlanState **planstates,
56+
Bitmapset **rels_used);
57+
static void ExplainPreScanSubPlans(List *plans, Bitmapset **rels_used);
5458
static void ExplainNode(PlanState *planstate, List *ancestors,
5559
const char *relationship, const char *plan_name,
5660
ExplainState *es);
@@ -539,9 +543,13 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
539543
void
540544
ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc)
541545
{
546+
Bitmapset *rels_used = NULL;
547+
542548
Assert(queryDesc->plannedstmt != NULL);
543549
es->pstmt = queryDesc->plannedstmt;
544550
es->rtable = queryDesc->plannedstmt->rtable;
551+
ExplainPreScanNode(queryDesc->planstate, &rels_used);
552+
es->rtable_names = select_rtable_names_for_explain(es->rtable, rels_used);
545553
ExplainNode(queryDesc->planstate, NIL, NULL, NULL, es);
546554
}
547555

@@ -640,6 +648,132 @@ elapsed_time(instr_time *starttime)
640648
return INSTR_TIME_GET_DOUBLE(endtime);
641649
}
642650

651+
/*
652+
* ExplainPreScanNode -
653+
* Prescan the planstate tree to identify which RTEs are referenced
654+
*
655+
* Adds the relid of each referenced RTE to *rels_used. The result controls
656+
* which RTEs are assigned aliases by select_rtable_names_for_explain. This
657+
* ensures that we don't confusingly assign un-suffixed aliases to RTEs that
658+
* never appear in the EXPLAIN output (such as inheritance parents).
659+
*/
660+
static void
661+
ExplainPreScanNode(PlanState *planstate, Bitmapset **rels_used)
662+
{
663+
Plan *plan = planstate->plan;
664+
665+
switch (nodeTag(plan))
666+
{
667+
case T_SeqScan:
668+
case T_IndexScan:
669+
case T_IndexOnlyScan:
670+
case T_BitmapHeapScan:
671+
case T_TidScan:
672+
case T_SubqueryScan:
673+
case T_FunctionScan:
674+
case T_ValuesScan:
675+
case T_CteScan:
676+
case T_WorkTableScan:
677+
case T_ForeignScan:
678+
*rels_used = bms_add_member(*rels_used,
679+
((Scan *) plan)->scanrelid);
680+
break;
681+
case T_ModifyTable:
682+
/* cf ExplainModifyTarget */
683+
*rels_used = bms_add_member(*rels_used,
684+
linitial_int(((ModifyTable *) plan)->resultRelations));
685+
break;
686+
default:
687+
break;
688+
}
689+
690+
/* initPlan-s */
691+
if (planstate->initPlan)
692+
ExplainPreScanSubPlans(planstate->initPlan, rels_used);
693+
694+
/* lefttree */
695+
if (outerPlanState(planstate))
696+
ExplainPreScanNode(outerPlanState(planstate), rels_used);
697+
698+
/* righttree */
699+
if (innerPlanState(planstate))
700+
ExplainPreScanNode(innerPlanState(planstate), rels_used);
701+
702+
/* special child plans */
703+
switch (nodeTag(plan))
704+
{
705+
case T_ModifyTable:
706+
ExplainPreScanMemberNodes(((ModifyTable *) plan)->plans,
707+
((ModifyTableState *) planstate)->mt_plans,
708+
rels_used);
709+
break;
710+
case T_Append:
711+
ExplainPreScanMemberNodes(((Append *) plan)->appendplans,
712+
((AppendState *) planstate)->appendplans,
713+
rels_used);
714+
break;
715+
case T_MergeAppend:
716+
ExplainPreScanMemberNodes(((MergeAppend *) plan)->mergeplans,
717+
((MergeAppendState *) planstate)->mergeplans,
718+
rels_used);
719+
break;
720+
case T_BitmapAnd:
721+
ExplainPreScanMemberNodes(((BitmapAnd *) plan)->bitmapplans,
722+
((BitmapAndState *) planstate)->bitmapplans,
723+
rels_used);
724+
break;
725+
case T_BitmapOr:
726+
ExplainPreScanMemberNodes(((BitmapOr *) plan)->bitmapplans,
727+
((BitmapOrState *) planstate)->bitmapplans,
728+
rels_used);
729+
break;
730+
case T_SubqueryScan:
731+
ExplainPreScanNode(((SubqueryScanState *) planstate)->subplan,
732+
rels_used);
733+
break;
734+
default:
735+
break;
736+
}
737+
738+
/* subPlan-s */
739+
if (planstate->subPlan)
740+
ExplainPreScanSubPlans(planstate->subPlan, rels_used);
741+
}
742+
743+
/*
744+
* Prescan the constituent plans of a ModifyTable, Append, MergeAppend,
745+
* BitmapAnd, or BitmapOr node.
746+
*
747+
* Note: we don't actually need to examine the Plan list members, but
748+
* we need the list in order to determine the length of the PlanState array.
749+
*/
750+
static void
751+
ExplainPreScanMemberNodes(List *plans, PlanState **planstates,
752+
Bitmapset **rels_used)
753+
{
754+
int nplans = list_length(plans);
755+
int j;
756+
757+
for (j = 0; j < nplans; j++)
758+
ExplainPreScanNode(planstates[j], rels_used);
759+
}
760+
761+
/*
762+
* Prescan a list of SubPlans (or initPlans, which also use SubPlan nodes).
763+
*/
764+
static void
765+
ExplainPreScanSubPlans(List *plans, Bitmapset **rels_used)
766+
{
767+
ListCell *lst;
768+
769+
foreach(lst, plans)
770+
{
771+
SubPlanState *sps = (SubPlanState *) lfirst(lst);
772+
773+
ExplainPreScanNode(sps->planstate, rels_used);
774+
}
775+
}
776+
643777
/*
644778
* ExplainNode -
645779
* Appends a description of a plan tree to es->str
@@ -1440,7 +1574,8 @@ show_plan_tlist(PlanState *planstate, List *ancestors, ExplainState *es)
14401574
/* Set up deparsing context */
14411575
context = deparse_context_for_planstate((Node *) planstate,
14421576
ancestors,
1443-
es->rtable);
1577+
es->rtable,
1578+
es->rtable_names);
14441579
useprefix = list_length(es->rtable) > 1;
14451580

14461581
/* Deparse each result column (we now include resjunk ones) */
@@ -1471,7 +1606,8 @@ show_expression(Node *node, const char *qlabel,
14711606
/* Set up deparsing context */
14721607
context = deparse_context_for_planstate((Node *) planstate,
14731608
ancestors,
1474-
es->rtable);
1609+
es->rtable,
1610+
es->rtable_names);
14751611

14761612
/* Deparse the expression */
14771613
exprstr = deparse_expression(node, context, useprefix, false);
@@ -1573,7 +1709,8 @@ show_sort_keys_common(PlanState *planstate, int nkeys, AttrNumber *keycols,
15731709
/* Set up deparsing context */
15741710
context = deparse_context_for_planstate((Node *) planstate,
15751711
ancestors,
1576-
es->rtable);
1712+
es->rtable,
1713+
es->rtable_names);
15771714
useprefix = (list_length(es->rtable) > 1 || es->verbose);
15781715

15791716
for (keyno = 0; keyno < nkeys; keyno++)
@@ -1813,8 +1950,10 @@ ExplainTargetRel(Plan *plan, Index rti, ExplainState *es)
18131950
char *namespace = NULL;
18141951
const char *objecttag = NULL;
18151952
RangeTblEntry *rte;
1953+
char *refname;
18161954

18171955
rte = rt_fetch(rti, es->rtable);
1956+
refname = (char *) list_nth(es->rtable_names, rti - 1);
18181957

18191958
switch (nodeTag(plan))
18201959
{
@@ -1887,18 +2026,18 @@ ExplainTargetRel(Plan *plan, Index rti, ExplainState *es)
18872026
quote_identifier(objectname));
18882027
else if (objectname != NULL)
18892028
appendStringInfo(es->str, " %s", quote_identifier(objectname));
1890-
if (objectname == NULL ||
1891-
strcmp(rte->eref->aliasname, objectname) != 0)
1892-
appendStringInfo(es->str, " %s",
1893-
quote_identifier(rte->eref->aliasname));
2029+
if (refname != NULL &&
2030+
(objectname == NULL || strcmp(refname, objectname) != 0))
2031+
appendStringInfo(es->str, " %s", quote_identifier(refname));
18942032
}
18952033
else
18962034
{
18972035
if (objecttag != NULL && objectname != NULL)
18982036
ExplainPropertyText(objecttag, objectname, es);
18992037
if (namespace != NULL)
19002038
ExplainPropertyText("Schema", namespace, es);
1901-
ExplainPropertyText("Alias", rte->eref->aliasname, es);
2039+
if (refname != NULL)
2040+
ExplainPropertyText("Alias", refname, es);
19022041
}
19032042
}
19042043

0 commit comments

Comments
 (0)