diff --git a/src/backend/distributed/executor/adaptive_executor.c b/src/backend/distributed/executor/adaptive_executor.c index 1453f992c..ad711720b 100644 --- a/src/backend/distributed/executor/adaptive_executor.c +++ b/src/backend/distributed/executor/adaptive_executor.c @@ -694,13 +694,7 @@ AdaptiveExecutor(CitusScanState *scanState) Assert(!scanState->finishedRemoteScan); /* Reset Task fields that are only valid for a single execution */ - Task *task = NULL; - foreach_ptr(task, taskList) - { - task->totalReceivedTupleData = 0; - task->fetchedExplainAnalyzePlacementIndex = 0; - task->fetchedExplainAnalyzePlan = NULL; - } + ResetExplainAnalyzeData(taskList); scanState->tuplestorestate = tuplestore_begin_heap(randomAccess, interTransactions, work_mem); diff --git a/src/backend/distributed/planner/multi_explain.c b/src/backend/distributed/planner/multi_explain.c index fa645bbd3..1eb3354b1 100644 --- a/src/backend/distributed/planner/multi_explain.c +++ b/src/backend/distributed/planner/multi_explain.c @@ -1194,8 +1194,26 @@ ExplainAnalyzeDestPutTuple(TupleDestination *self, Task *task, char *fetchedExplainAnalyzePlan = TextDatumGetCString(explainAnalyze); + /* + * Allocate fetchedExplainAnalyzePlan in the same context as the Task, since we are + * currently in execution context and a Task can span multiple executions. + * + * Although we won't reuse the same value in a future execution, but we have + * calls to CheckNodeCopyAndSerialization() which asserts copy functions of the task + * work as expected, which will try to copy this value in a future execution. + * + * Why we don't we just allocate this field in executor context and reset it before + * the next execution? Because when an error is raised we can skip pretty much most + * of the meaningful places that we can insert the reset. + * + * TODO: Take all EXPLAIN ANALYZE related fields out of Task and store them in a + * Task to ExplainAnalyzePrivate mapping in multi_explain.c, so we don't need to + * do these hacky memory context management tricks. + */ + MemoryContext taskContext = GetMemoryChunkContext(tupleDestination->originalTask); + tupleDestination->originalTask->fetchedExplainAnalyzePlan = - pstrdup(fetchedExplainAnalyzePlan); + MemoryContextStrdup(taskContext, fetchedExplainAnalyzePlan); tupleDestination->originalTask->fetchedExplainAnalyzePlacementIndex = placementIndex; } @@ -1207,6 +1225,27 @@ ExplainAnalyzeDestPutTuple(TupleDestination *self, Task *task, } +/* + * ResetExplainAnalyzeData reset fields in Task that are used by multi_explain.c + */ +void +ResetExplainAnalyzeData(List *taskList) +{ + Task *task = NULL; + foreach_ptr(task, taskList) + { + if (task->fetchedExplainAnalyzePlan != NULL) + { + pfree(task->fetchedExplainAnalyzePlan); + } + + task->totalReceivedTupleData = 0; + task->fetchedExplainAnalyzePlacementIndex = 0; + task->fetchedExplainAnalyzePlan = NULL; + } +} + + /* * ExplainAnalyzeDestTupleDescForQuery implements TupleDestination->tupleDescForQuery * for ExplainAnalyzeDestination. diff --git a/src/include/distributed/multi_explain.h b/src/include/distributed/multi_explain.h index 67e7e4641..01c07b4ad 100644 --- a/src/include/distributed/multi_explain.h +++ b/src/include/distributed/multi_explain.h @@ -26,5 +26,6 @@ extern List * ExplainAnalyzeTaskList(List *originalTaskList, TupleDestination *defaultTupleDest, TupleDesc tupleDesc, ParamListInfo params); extern bool RequestedForExplainAnalyze(CitusScanState *node); +extern void ResetExplainAnalyzeData(List *taskList); #endif /* MULTI_EXPLAIN_H */