From 0f28a1197001fff5b3612ba6413b5f9bb43b4426 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sun, 13 Nov 2016 13:22:52 -0800 Subject: [PATCH 1/3] Remove citus.explain_multi_logical/physical_plan. They make fixing explain for prepared statement harder, and they don't really fit into EXPLAIN in the first place. Additionally they're currently not exercised in any tests. --- .../distributed/planner/multi_explain.c | 16 ------------- src/backend/distributed/shared_library_init.c | 24 ------------------- src/include/distributed/multi_explain.h | 2 -- 3 files changed, 42 deletions(-) diff --git a/src/backend/distributed/planner/multi_explain.c b/src/backend/distributed/planner/multi_explain.c index 0e3e90b33..b44c59f57 100644 --- a/src/backend/distributed/planner/multi_explain.c +++ b/src/backend/distributed/planner/multi_explain.c @@ -52,8 +52,6 @@ /* Config variables that enable printing distributed query plans */ -bool ExplainMultiLogicalPlan = false; -bool ExplainMultiPhysicalPlan = false; bool ExplainDistributedQueries = true; bool ExplainAllTasks = false; @@ -185,24 +183,10 @@ MultiExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, INSTR_TIME_SET_CURRENT(planDuration); INSTR_TIME_SUBTRACT(planDuration, planStart); - if (ExplainMultiLogicalPlan) { - MultiTreeRoot *multiTree = MultiLogicalPlanCreate(query); - char *logicalPlanString = CitusNodeToString(multiTree); - char *formattedPlanString = pretty_format_node_dump(logicalPlanString); - appendStringInfo(es->str, "logical plan:\n"); - appendStringInfo(es->str, "%s\n", formattedPlanString); } - if (ExplainMultiPhysicalPlan) - { - char *physicalPlanString = CitusNodeToString(multiPlan); - char *formattedPlanString = pretty_format_node_dump(physicalPlanString); - - appendStringInfo(es->str, "physical plan:\n"); - appendStringInfo(es->str, "%s\n", formattedPlanString); - } if (!ExplainDistributedQueries) { diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index 509ab0381..102016c8a 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -295,30 +295,6 @@ RegisterCitusConfigVariables(void) 0, NULL, NULL, NULL); - DefineCustomBoolVariable( - "citus.explain_multi_logical_plan", - gettext_noop("Enables Explain to print out distributed logical plans."), - gettext_noop("We use this private configuration entry as a debugging aid. " - "If enabled, the Explain command prints out the optimized " - "logical plan for distributed queries."), - &ExplainMultiLogicalPlan, - false, - PGC_USERSET, - GUC_NO_SHOW_ALL, - NULL, NULL, NULL); - - DefineCustomBoolVariable( - "citus.explain_multi_physical_plan", - gettext_noop("Enables Explain to print out distributed physical plans."), - gettext_noop("We use this private configuration entry as a debugging aid. " - "If enabled, the Explain command prints out the physical " - "plan for distributed queries."), - &ExplainMultiPhysicalPlan, - false, - PGC_USERSET, - GUC_NO_SHOW_ALL, - NULL, NULL, NULL); - DefineCustomBoolVariable( "citus.explain_distributed_queries", gettext_noop("Enables Explain for distributed queries."), diff --git a/src/include/distributed/multi_explain.h b/src/include/distributed/multi_explain.h index 984b8e2ab..55f4bf75d 100644 --- a/src/include/distributed/multi_explain.h +++ b/src/include/distributed/multi_explain.h @@ -13,8 +13,6 @@ #include "executor/executor.h" /* Config variables managed via guc.c to explain distributed query plans */ -extern bool ExplainMultiLogicalPlan; -extern bool ExplainMultiPhysicalPlan; extern bool ExplainDistributedQueries; extern bool ExplainAllTasks; From 608bed03874ce597b69b8df7cd474376a656b284 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 19 Jan 2017 15:09:13 -0800 Subject: [PATCH 2/3] Don't duplicate planning logic in citus' explain hook. Instead use pg_plan_query() like the normal explain does, and use that to explain the query. That's important because it allows to remove the duplicated planner logic from multi_explain - and that logic is about to get more complicated. --- .../distributed/planner/multi_explain.c | 109 +++++++----------- 1 file changed, 43 insertions(+), 66 deletions(-) diff --git a/src/backend/distributed/planner/multi_explain.c b/src/backend/distributed/planner/multi_explain.c index b44c59f57..fd4f59cef 100644 --- a/src/backend/distributed/planner/multi_explain.c +++ b/src/backend/distributed/planner/multi_explain.c @@ -77,6 +77,9 @@ static void ExplainTask(Task *task, int placementIndex, List *explainOutputList, static void ExplainTaskPlacement(ShardPlacement *taskPlacement, List *explainOutputList, ExplainState *es); static StringInfo BuildRemoteExplainQuery(char *queryString, ExplainState *es); +static void MultiExplainOnePlan(PlannedStmt *plan, IntoClause *into, + ExplainState *es, const char *queryString, + ParamListInfo params, const instr_time *planDuration); /* Static Explain functions copied from explain.c */ static void ExplainOpenGroup(const char *objtype, const char *labelname, @@ -98,17 +101,10 @@ void MultiExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, const char *queryString, ParamListInfo params) { - MultiPlan *multiPlan = NULL; - CmdType commandType = CMD_UNKNOWN; - PlannedStmt *initialPlan = NULL; - Job *workerJob = NULL; - bool routerExecutablePlan = false; instr_time planStart; instr_time planDuration; - Query *originalQuery = NULL; - RelationRestrictionContext *restrictionContext = NULL; - bool localQuery = !NeedsDistributedPlanning(query); int cursorOptions = 0; + PlannedStmt *plan = NULL; #if PG_VERSION_NUM >= 90600 @@ -122,71 +118,54 @@ MultiExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, } #endif - /* handle local queries in the same way as ExplainOneQuery */ - if (localQuery) - { - PlannedStmt *plan = NULL; - - INSTR_TIME_SET_CURRENT(planStart); - - /* plan the query */ - plan = pg_plan_query(query, cursorOptions, params); - - INSTR_TIME_SET_CURRENT(planDuration); - INSTR_TIME_SUBTRACT(planDuration, planStart); - - /* run it (if needed) and produce output */ - ExplainOnePlan(plan, into, es, queryString, params, &planDuration); - - return; - } - - /* - * standard_planner scribbles on it's input, but for deparsing we need the - * unmodified form. So copy once we're sure it's a distributed query. - */ - originalQuery = copyObject(query); - - /* measure the full planning time to display in EXPLAIN ANALYZE */ + /* plan query, just like ExplainOneQuery does */ INSTR_TIME_SET_CURRENT(planStart); - restrictionContext = CreateAndPushRestrictionContext(); - - PG_TRY(); - { - /* call standard planner to modify the query structure before multi planning */ - initialPlan = standard_planner(query, cursorOptions, params); - - commandType = initialPlan->commandType; - if (commandType == CMD_INSERT || commandType == CMD_UPDATE || - commandType == CMD_DELETE) - { - if (es->analyze) - { - ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("Using ANALYZE for INSERT/UPDATE/DELETE on " - "distributed tables is not supported."))); - } - } - - multiPlan = CreatePhysicalPlan(originalQuery, query, restrictionContext); - } - PG_CATCH(); - { - PopRestrictionContext(); - PG_RE_THROW(); - } - PG_END_TRY(); - - PopRestrictionContext(); + /* plan the query */ + plan = pg_plan_query(query, cursorOptions, params); INSTR_TIME_SET_CURRENT(planDuration); INSTR_TIME_SUBTRACT(planDuration, planStart); + /* if not a distributed query, use plain explain infrastructure */ + if (!HasCitusToplevelNode(plan)) { + /* run it (if needed) and produce output */ + ExplainOnePlan(plan, into, es, queryString, params, &planDuration); + } + else + { + MultiExplainOnePlan(plan, into, es, queryString, params, &planDuration); + } +} + +/* + * MultiExplainOnePlan explains the plan for an individual distributed query. + */ +static void +MultiExplainOnePlan(PlannedStmt *plan, IntoClause *into, + ExplainState *es, const char *queryString, + ParamListInfo params, const instr_time *planDuration) +{ + MultiPlan *multiPlan = NULL; + CmdType commandType = CMD_UNKNOWN; + Job *workerJob = NULL; + bool routerExecutablePlan = false; + + commandType = plan->commandType; + if (commandType == CMD_INSERT || commandType == CMD_UPDATE || + commandType == CMD_DELETE) + { + if (es->analyze) + { + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("Using ANALYZE for INSERT/UPDATE/DELETE on " + "distributed tables is not supported."))); + } } + multiPlan = GetMultiPlan(plan); if (!ExplainDistributedQueries) { @@ -252,8 +231,6 @@ MultiExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, if (!routerExecutablePlan) { - PlannedStmt *masterPlan = MultiQueryContainerNode(initialPlan, multiPlan); - if (es->format == EXPLAIN_FORMAT_TEXT) { appendStringInfoSpaces(es->str, es->indent * 2); @@ -263,7 +240,7 @@ MultiExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, ExplainOpenGroup("Master Query", "Master Query", false, es); - ExplainMasterPlan(masterPlan, into, es, queryString, params, &planDuration); + ExplainMasterPlan(plan, into, es, queryString, params, planDuration); ExplainCloseGroup("Master Query", "Master Query", false, es); From 3a36d32c43c58592f81c405f648b17125ca0f6ce Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 18 Jan 2017 16:43:35 -0800 Subject: [PATCH 3/3] Mark some now unnecessarily exposed multi_planner.c functions static. --- src/backend/distributed/planner/multi_planner.c | 13 +++++++++---- src/include/distributed/multi_planner.h | 8 -------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/backend/distributed/planner/multi_planner.c b/src/backend/distributed/planner/multi_planner.c index d72f9db0c..c21665d9c 100644 --- a/src/backend/distributed/planner/multi_planner.c +++ b/src/backend/distributed/planner/multi_planner.c @@ -36,10 +36,15 @@ static List *relationRestrictionContextList = NIL; /* local function forward declarations */ static void CheckNodeIsDumpable(Node *node); - - -/* local function forward declarations */ static char * GetMultiPlanString(PlannedStmt *result); +static PlannedStmt * MultiQueryContainerNode(PlannedStmt *result, + struct MultiPlan *multiPlan); +static struct MultiPlan * CreatePhysicalPlan(Query *originalQuery, Query *query, + RelationRestrictionContext * + restrictionContext); +static RelationRestrictionContext * CreateAndPushRestrictionContext(void); +static RelationRestrictionContext * CurrentRestrictionContext(void); +static void PopRestrictionContext(void); /* Distributed planner hook */ @@ -123,7 +128,7 @@ multi_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) * target shards. SELECT queries go through the full logical plan/optimize/ * physical plan process needed to produce distributed query plans. */ -MultiPlan * +static MultiPlan * CreatePhysicalPlan(Query *originalQuery, Query *query, RelationRestrictionContext *restrictionContext) { diff --git a/src/include/distributed/multi_planner.h b/src/include/distributed/multi_planner.h index 7c96b55bf..a425232fa 100644 --- a/src/include/distributed/multi_planner.h +++ b/src/include/distributed/multi_planner.h @@ -53,16 +53,8 @@ extern PlannedStmt * multi_planner(Query *parse, int cursorOptions, extern bool HasCitusToplevelNode(PlannedStmt *planStatement); struct MultiPlan; -extern struct MultiPlan * CreatePhysicalPlan(Query *originalQuery, Query *query, - RelationRestrictionContext * - restrictionContext); extern struct MultiPlan * GetMultiPlan(PlannedStmt *planStatement); -extern PlannedStmt * MultiQueryContainerNode(PlannedStmt *result, - struct MultiPlan *multiPlan); extern void multi_relation_restriction_hook(PlannerInfo *root, RelOptInfo *relOptInfo, Index index, RangeTblEntry *rte); -extern RelationRestrictionContext * CreateAndPushRestrictionContext(void); -extern RelationRestrictionContext * CurrentRestrictionContext(void); -extern void PopRestrictionContext(void); #endif /* MULTI_PLANNER_H */