address review

src/backend/distributed/planner/recursive_planning.c

@@ -147,7 +147,8 @@ static bool ShouldRecursivelyPlanNonColocatedSubqueries(Query *subquery,
 														RecursivePlanningContext *
 														context);
 static bool ShouldConvertLocalTableJoinsToSubqueries(List *rangeTableList,
-													 List *tableList, List *rtableList);
+													 List *referencedRteList,
+													 List *rtableList);
 
 static bool ContainsSubquery(Query *query);
 static void RecursivelyPlanNonColocatedSubqueries(Query *subquery,
@@ -200,9 +201,7 @@ static Query * CreateOuterSubquery(RangeTblEntry *rangeTableEntry,
 								   List *outerSubqueryTargetList);
 static List * GenerateRequiredColNamesFromTargetList(List *targetList);
 static char * GetRelationNameAndAliasName(RangeTblEntry *rangeTablentry);
-static void ContainsLocalTableDistributedTableVars(List *tableList, List *rtableList,
+static bool ContainsDistributedTable(List *rteList);
-												   bool *containsLocalTable,
-												   bool *containsDistributedTable);
 
 /*
  * GenerateSubplansForSubqueriesAndCTEs is a wrapper around RecursivelyPlanSubqueriesAndCTEs.
@@ -361,8 +360,8 @@ RecursivelyPlanSubqueriesAndCTEs(Query *query, RecursivePlanningContext *context
 		RecursivelyPlanNonColocatedSubqueries(query, context);
 	}
 
-	List *tableList = PullAllRangeTablesEntries(query, context->rtableList);
+	List *rteList = PullAllRangeTablesEntries(query, context->rtableList);
-	if (ShouldConvertLocalTableJoinsToSubqueries(query->rtable, tableList,
+	if (ShouldConvertLocalTableJoinsToSubqueries(query->rtable, rteList,
 												 context->rtableList))
 	{
 		/*
@@ -383,7 +382,7 @@ RecursivelyPlanSubqueriesAndCTEs(Query *query, RecursivePlanningContext *context
  * convert local-dist table joins to subqueries.
  */
 static bool
-ShouldConvertLocalTableJoinsToSubqueries(List *rangeTableList, List *tableList,
+ShouldConvertLocalTableJoinsToSubqueries(List *rangeTableList, List *referencedRteList,
 										 List *rtableList)
 {
 	if (LocalTableJoinPolicy == LOCAL_JOIN_POLICY_NEVER)
@@ -401,33 +400,41 @@ ShouldConvertLocalTableJoinsToSubqueries(List *rangeTableList, List *tableList,
 		return true;
 	}
 
-	/* if we have vars that references local or distributed tables, it means we need to do subquery-conversion */
+	if (!containsLocalTable)
-	ContainsLocalTableDistributedTableVars(tableList, rtableList, &containsLocalTable,
+	{
-										   &containsDistributedTable);
+		/* if there is no local table, we don't need to do any conversion */
+		return false;
+	}
 
-	return containsDistributedTable && containsLocalTable;
+	/*
+	 * if referenced table list of this query contains a distributed table
+	 * we need to do a convertion
+	 */
+	if (ContainsDistributedTable(referencedRteList))
+	{
+		return true;
+	}
+
+	return false;
 }
 
 
-static void
+/*
-ContainsLocalTableDistributedTableVars(List *tableList, List *rtableList,
+ * ContainsDistributedTable returns true if the given rteList contains
-									   bool *containsLocalTable,
+ * a distributed table rte.
-									   bool *containsDistributedTable)
+ */
+static bool
+ContainsDistributedTable(List *rteList)
 {
 	RangeTblEntry *rte = NULL;
-	foreach_ptr(rte, tableList)
+	foreach_ptr(rte, rteList)
 	{
 		if (IsDistributedOrReferenceTableRTE((Node *) rte))
 		{
-			*containsDistributedTable = true;
+			return true;
-		}
-		else if (IsRecursivelyPlannableRelation(rte) &&
-				 IsLocalTableRteOrMatView((Node *) rte))
-		{
-			/* we consider citus local tables as local table */
-			*containsLocalTable = true;
 		}
 	}
+	return false;
 }
 
 
@@ -1797,7 +1804,6 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry)
 			subquery->targetList = lappend(subquery->targetList, targetEntry);
 		}
 	}
-
 	/*
 	 * If tupleDesc is NULL we have 2 different cases:
 	 *
@@ -1847,7 +1853,6 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry)
 				columnType = list_nth_oid(rangeTblFunction->funccoltypes,
 										  targetColumnIndex);
 			}
-
 			/* use the types in the function definition otherwise */
 			else
 			{

src/backend/distributed/utils/var_utils.c

@@ -31,8 +31,8 @@
 
 typedef struct PullTableContext
 {
-	List *rteList;
+	List *referencedRteList; /* stores all the RTEs that are referenced by a var */
-	List *rtableList;
+	List *rtableList; /* contains rtable's of upper queries, including the current query */
 }PullTableContext;
 
 
@@ -51,14 +51,14 @@ PullAllRangeTablesEntries(Query *query, List *rtableList)
 {
 	PullTableContext p;
 	p.rtableList = rtableList;
-	p.rteList = NIL;
+	p.referencedRteList = NIL;
 
 	query_or_expression_tree_walker((Node *) query,
 									PullAllRangeTablesEntriesWalker,
 									(void *) &p,
 									0);
 
-	return p.rteList;
+	return p.referencedRteList;
 }
 
 
@@ -73,7 +73,7 @@ PullAllRangeTablesEntriesWalker(Node *node, PullTableContext *pullTableContext)
 	{
 		return false;
 	}
-	if (IsA(node, Var))
+	else if (IsA(node, Var))
 	{
 		Var *var = (Var *) node;
 
@@ -86,17 +86,18 @@ PullAllRangeTablesEntriesWalker(Node *node, PullTableContext *pullTableContext)
 		}
 		List *rtable = (List *) list_nth(rtableList, index);
 		RangeTblEntry *rte = (RangeTblEntry *) list_nth(rtable, var->varno - 1);
-		pullTableContext->rteList = lappend(pullTableContext->rteList, rte);
+		pullTableContext->referencedRteList = lappend(pullTableContext->referencedRteList,
+													  rte);
 		return false;
 	}
-	if (IsA(node, PlaceHolderVar))
+	else if (IsA(node, PlaceHolderVar))
 	{
 		/* PlaceHolderVar *phv = (PlaceHolderVar *) node; */
 
 		/* we don't want to look into the contained expression */
 		return false;
 	}
-	if (IsA(node, Query))
+	else if (IsA(node, Query))
 	{
 		/* Recurse into RTE subquery or not-yet-planned sublink subquery */
 
@@ -107,6 +108,9 @@ PullAllRangeTablesEntriesWalker(Node *node, PullTableContext *pullTableContext)
 		pullTableContext->rtableList = list_delete_first(pullTableContext->rtableList);
 		return result;
 	}
+	else
+	{
 		return expression_tree_walker(node, PullAllRangeTablesEntriesWalker,
 									  (void *) pullTableContext);
 	}
+}