Skip to content

Commit 92f0734

Browse files
author
Svetlana Derevyanko
committed
[PGPRO-7630] Post-processing for nodes added in plan tree by pathman
New nodes added in pathman planner hook had no correct plan_node_id, which could cause problems later for statistics collector. Added fixes for 'custom_scan_tlist' to let EXPLAIN (VERBOSE) work. Also changed queryId type on uint64. Added hook for compatibility with pgpro_stats. Fixed tree walkers for ModifyTable. Tags: pg_pathman
1 parent 06a5e02 commit 92f0734

8 files changed

+137
-23
lines changed

Diff for: src/hooks.c

+80-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "hooks.h"
2222
#include "init.h"
2323
#include "partition_filter.h"
24+
#include "partition_overseer.h"
2425
#include "partition_router.h"
2526
#include "pathman_workers.h"
2627
#include "planner_tree_modification.h"
@@ -74,6 +75,7 @@ planner_hook_type pathman_planner_hook_next = NULL;
7475
post_parse_analyze_hook_type pathman_post_parse_analyze_hook_next = NULL;
7576
shmem_startup_hook_type pathman_shmem_startup_hook_next = NULL;
7677
ProcessUtility_hook_type pathman_process_utility_hook_next = NULL;
78+
ExecutorStart_hook_type pathman_executor_start_hook_prev = NULL;
7779

7880

7981
/* Take care of joins */
@@ -673,6 +675,23 @@ execute_for_plantree(PlannedStmt *planned_stmt,
673675
planned_stmt->subplans = subplans;
674676
}
675677

678+
/*
679+
* Truncated version of set_plan_refs.
680+
* Pathman can add nodes to already completed and post-processed plan tree.
681+
* reset_plan_node_ids fixes some presentation values for updated plan tree
682+
* to avoid problems in further processing.
683+
*/
684+
static Plan *
685+
reset_plan_node_ids(Plan *plan, void *lastPlanNodeId)
686+
{
687+
if (plan == NULL)
688+
return NULL;
689+
690+
plan->plan_node_id = (*(int *) lastPlanNodeId)++;
691+
692+
return plan;
693+
}
694+
676695
/*
677696
* Planner hook. It disables inheritance for tables that have been partitioned
678697
* by pathman to prevent standart PostgreSQL partitioning mechanism from
@@ -688,7 +707,7 @@ pathman_planner_hook(Query *parse, int cursorOptions, ParamListInfo boundParams)
688707
#endif
689708
{
690709
PlannedStmt *result;
691-
uint32 query_id = parse->queryId;
710+
uint64 query_id = parse->queryId;
692711

693712
/* Save the result in case it changes */
694713
bool pathman_ready = IsPathmanReady();
@@ -720,6 +739,9 @@ pathman_planner_hook(Query *parse, int cursorOptions, ParamListInfo boundParams)
720739

721740
if (pathman_ready)
722741
{
742+
int lastPlanNodeId = 0;
743+
ListCell *l;
744+
723745
/* Add PartitionFilter node for INSERT queries */
724746
execute_for_plantree(result, add_partition_filters);
725747

@@ -729,6 +751,13 @@ pathman_planner_hook(Query *parse, int cursorOptions, ParamListInfo boundParams)
729751
/* Decrement planner() calls count */
730752
decr_planner_calls_count();
731753

754+
/* remake parsed tree presentation fixes due to possible adding nodes */
755+
result->planTree = plan_tree_visitor(result->planTree, reset_plan_node_ids, &lastPlanNodeId);
756+
foreach(l, result->subplans)
757+
{
758+
lfirst(l) = plan_tree_visitor((Plan *) lfirst(l), reset_plan_node_ids, &lastPlanNodeId);
759+
}
760+
732761
/* HACK: restore queryId set by pg_stat_statements */
733762
result->queryId = query_id;
734763
}
@@ -1125,3 +1154,53 @@ pathman_process_utility_hook(Node *first_arg,
11251154
dest, completionTag);
11261155
#endif
11271156
}
1157+
1158+
/*
1159+
* Planstate tree nodes could have been copied.
1160+
* It breaks references on correspoding
1161+
* ModifyTable node from PartitionRouter nodes.
1162+
*/
1163+
static void
1164+
fix_mt_refs(PlanState *state, void *context)
1165+
{
1166+
ModifyTableState *mt_state = (ModifyTableState *) state;
1167+
PartitionRouterState *pr_state;
1168+
#if PG_VERSION_NUM < 140000
1169+
int i;
1170+
#endif
1171+
1172+
if (!IsA(state, ModifyTableState))
1173+
return;
1174+
#if PG_VERSION_NUM >= 140000
1175+
{
1176+
CustomScanState *pf_state = (CustomScanState *) outerPlanState(mt_state);
1177+
#else
1178+
for (i = 0; i < mt_state->mt_nplans; i++)
1179+
{
1180+
CustomScanState *pf_state = (CustomScanState *) mt_state->mt_plans[i];
1181+
#endif
1182+
if (IsPartitionFilterState(pf_state))
1183+
{
1184+
pr_state = linitial(pf_state->custom_ps);
1185+
if (IsPartitionRouterState(pr_state))
1186+
{
1187+
pr_state->mt_state = mt_state;
1188+
}
1189+
}
1190+
}
1191+
}
1192+
1193+
void
1194+
pathman_executor_start_hook(QueryDesc *queryDesc, int eflags)
1195+
{
1196+
if (pathman_executor_start_hook_prev)
1197+
pathman_executor_start_hook_prev(queryDesc, eflags);
1198+
else
1199+
standard_ExecutorStart(queryDesc, eflags);
1200+
1201+
/*
1202+
* HACK for compatibility with pgpro_stats.
1203+
* Fix possibly broken planstate tree.
1204+
*/
1205+
state_tree_visitor(queryDesc->planstate, fix_mt_refs, NULL);
1206+
}

Diff for: src/include/hooks.h

+3
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ extern post_parse_analyze_hook_type pathman_post_parse_analyze_hook_next;
2828
extern shmem_startup_hook_type pathman_shmem_startup_hook_next;
2929
extern ProcessUtility_hook_type pathman_process_utility_hook_next;
3030
extern ExecutorRun_hook_type pathman_executor_run_hook_next;
31+
extern ExecutorStart_hook_type pathman_executor_start_hook_prev;
3132

3233

3334
void pathman_join_pathlist_hook(PlannerInfo *root,
@@ -115,4 +116,6 @@ void pathman_executor_hook(QueryDesc *queryDesc,
115116
ExecutorRun_CountArgType count);
116117
#endif
117118

119+
void pathman_executor_start_hook(QueryDesc *queryDescc,
120+
int eflags);
118121
#endif /* PATHMAN_HOOKS_H */

Diff for: src/include/partition_filter.h

+1
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ void destroy_tuple_map(TupleConversionMap *tuple_map);
183183

184184
List * pfilter_build_tlist(Plan *subplan);
185185

186+
void pfilter_tlist_fix_resjunk(CustomScan *subplan);
186187

187188
/* Find suitable partition using 'value' */
188189
Oid * find_partitions_for_value(Datum value, Oid value_type,

Diff for: src/include/partition_router.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ void partition_router_explain(CustomScanState *node,
7878
List *ancestors,
7979
ExplainState *es);
8080

81-
Plan *make_partition_router(Plan *subplan, int epq_param);
81+
Plan *make_partition_router(Plan *subplan, int epq_param, Index parent_rti);
8282
Node *partition_router_create_scan_state(CustomScan *node);
8383
TupleTableSlot *partition_router_exec(CustomScanState *node);
8484

Diff for: src/partition_filter.c

+29
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,7 @@ make_partition_filter(Plan *subplan,
817817
/* Prepare 'custom_scan_tlist' for EXPLAIN (VERBOSE) */
818818
cscan->custom_scan_tlist = copyObject(cscan->scan.plan.targetlist);
819819
ChangeVarNodes((Node *) cscan->custom_scan_tlist, INDEX_VAR, parent_rti, 0);
820+
pfilter_tlist_fix_resjunk(cscan);
820821

821822
/* Pack partitioned table's Oid and conflict_action */
822823
cscan->custom_private = list_make4(makeInteger(parent_relid),
@@ -1114,6 +1115,34 @@ pfilter_build_tlist(Plan *subplan)
11141115
return result_tlist;
11151116
}
11161117

1118+
/*
1119+
* resjunk Vars had its varattnos being set on nonexisting relation columns.
1120+
* For future processing service attributes should be indicated correctly.
1121+
*/
1122+
void
1123+
pfilter_tlist_fix_resjunk(CustomScan *css)
1124+
{
1125+
ListCell *lc;
1126+
1127+
foreach(lc, css->custom_scan_tlist)
1128+
{
1129+
TargetEntry *tle = (TargetEntry *) lfirst(lc);
1130+
1131+
if (!IsA(tle->expr, Const))
1132+
{
1133+
Var *var = (Var *) tle->expr;
1134+
1135+
if (tle->resjunk)
1136+
{
1137+
/* To make Var recognizable as service attribute. */
1138+
var->varattno = -1;
1139+
}
1140+
}
1141+
}
1142+
1143+
return;
1144+
}
1145+
11171146
/*
11181147
* ----------------------------------------------
11191148
* Additional init steps for ResultPartsStorage

Diff for: src/partition_router.c

+5-3
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ init_partition_router_static_data(void)
115115
}
116116

117117
Plan *
118-
make_partition_router(Plan *subplan, int epq_param)
118+
make_partition_router(Plan *subplan, int epq_param, Index parent_rti)
119119
{
120120
CustomScan *cscan = makeNode(CustomScan);
121121

@@ -136,8 +136,10 @@ make_partition_router(Plan *subplan, int epq_param)
136136
/* Build an appropriate target list */
137137
cscan->scan.plan.targetlist = pfilter_build_tlist(subplan);
138138

139-
/* FIXME: should we use the same tlist? */
140-
cscan->custom_scan_tlist = subplan->targetlist;
139+
/* Fix 'custom_scan_tlist' for EXPLAIN (VERBOSE) */
140+
cscan->custom_scan_tlist = copyObject(cscan->scan.plan.targetlist);
141+
ChangeVarNodes((Node *) cscan->custom_scan_tlist, INDEX_VAR, parent_rti, 0);
142+
pfilter_tlist_fix_resjunk(cscan);
141143

142144
return &cscan->scan.plan;
143145
}

Diff for: src/pg_pathman.c

+2
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,8 @@ _PG_init(void)
357357
planner_hook = pathman_planner_hook;
358358
pathman_process_utility_hook_next = ProcessUtility_hook;
359359
ProcessUtility_hook = pathman_process_utility_hook;
360+
pathman_executor_start_hook_prev = ExecutorStart_hook;
361+
ExecutorStart_hook = pathman_executor_start_hook;
360362

361363
/* Initialize static data for all subsystems */
362364
init_main_pathman_toggles();

Diff for: src/planner_tree_modification.c

+16-18
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ static void handle_modification_query(Query *parse, transform_query_cxt *context
122122
static Plan *partition_filter_visitor(Plan *plan, void *context);
123123
static Plan *partition_router_visitor(Plan *plan, void *context);
124124

125-
static void state_visit_subplans(List *plans, void (*visitor) (), void *context);
126-
static void state_visit_members(PlanState **planstates, int nplans, void (*visitor) (), void *context);
125+
static void state_visit_subplans(List *plans, void (*visitor) (PlanState *plan, void *context), void *context);
126+
static void state_visit_members(PlanState **planstates, int nplans, void (*visitor) (PlanState *plan, void *context), void *context);
127127

128128
static Oid find_deepest_partition(Oid relid, Index rti, Expr *quals);
129129
static Node *eval_extern_params_mutator(Node *node, ParamListInfo params);
@@ -137,13 +137,13 @@ static bool modifytable_contains_fdw(List *rtable, ModifyTable *node);
137137
* id in order to recognize them properly.
138138
*/
139139
#define QUERY_ID_INITIAL 0
140-
static uint32 latest_query_id = QUERY_ID_INITIAL;
140+
static uint64 latest_query_id = QUERY_ID_INITIAL;
141141

142142

143143
void
144144
assign_query_id(Query *query)
145145
{
146-
uint32 prev_id = latest_query_id++;
146+
uint64 prev_id = latest_query_id++;
147147

148148
if (prev_id > latest_query_id)
149149
elog(WARNING, "assign_query_id(): queryId overflow");
@@ -187,14 +187,12 @@ plan_tree_visitor(Plan *plan,
187187
plan_tree_visitor((Plan *) lfirst(l), visitor, context);
188188
break;
189189

190+
#if PG_VERSION_NUM < 140000 /* reworked in commit 86dc90056dfd */
190191
case T_ModifyTable:
191-
#if PG_VERSION_NUM >= 140000 /* reworked in commit 86dc90056dfd */
192-
plan_tree_visitor(outerPlan(plan), visitor, context);
193-
#else
194192
foreach (l, ((ModifyTable *) plan)->plans)
195193
plan_tree_visitor((Plan *) lfirst(l), visitor, context);
196-
#endif
197194
break;
195+
#endif
198196

199197
case T_Append:
200198
foreach (l, ((Append *) plan)->appendplans)
@@ -254,15 +252,13 @@ state_tree_visitor(PlanState *state,
254252
state_tree_visitor((PlanState *) lfirst(lc), visitor, context);
255253
break;
256254

255+
#if PG_VERSION_NUM < 140000 /* reworked in commit 86dc90056dfd */
257256
case T_ModifyTable:
258-
#if PG_VERSION_NUM >= 140000 /* reworked in commit 86dc90056dfd */
259-
visitor(outerPlanState(state), context);
260-
#else
261257
state_visit_members(((ModifyTableState *) state)->mt_plans,
262258
((ModifyTableState *) state)->mt_nplans,
263259
visitor, context);
264-
#endif
265260
break;
261+
#endif
266262

267263
case T_Append:
268264
state_visit_members(((AppendState *) state)->appendplans,
@@ -307,15 +303,15 @@ state_tree_visitor(PlanState *state,
307303
*/
308304
static void
309305
state_visit_subplans(List *plans,
310-
void (*visitor) (),
306+
void (*visitor) (PlanState *plan, void *context),
311307
void *context)
312308
{
313309
ListCell *lc;
314310

315311
foreach (lc, plans)
316312
{
317313
SubPlanState *sps = lfirst_node(SubPlanState, lc);
318-
visitor(sps->planstate, context);
314+
state_tree_visitor(sps->planstate, visitor, context);
319315
}
320316
}
321317

@@ -325,12 +321,12 @@ state_visit_subplans(List *plans,
325321
*/
326322
static void
327323
state_visit_members(PlanState **planstates, int nplans,
328-
void (*visitor) (), void *context)
324+
void (*visitor) (PlanState *plan, void *context), void *context)
329325
{
330326
int i;
331327

332328
for (i = 0; i < nplans; i++)
333-
visitor(planstates[i], context);
329+
state_tree_visitor(planstates[i], visitor, context);
334330
}
335331

336332

@@ -939,10 +935,12 @@ partition_router_visitor(Plan *plan, void *context)
939935

940936
#if PG_VERSION_NUM >= 140000 /* for changes 86dc90056dfd */
941937
prouter = make_partition_router(subplan,
942-
modify_table->epqParam);
938+
modify_table->epqParam,
939+
modify_table->nominalRelation);
943940
#else
944941
prouter = make_partition_router((Plan *) lfirst(lc1),
945-
modify_table->epqParam);
942+
modify_table->epqParam,
943+
modify_table->nominalRelation);
946944
#endif
947945

948946
pfilter = make_partition_filter((Plan *) prouter, relid,

0 commit comments

Comments
 (0)