From 73ef40886b0b789cb127fa33a567590fd99d0e17 Mon Sep 17 00:00:00 2001 From: SaitTalhaNisanci Date: Tue, 11 Aug 2020 15:01:23 +0300 Subject: [PATCH] Rename FindNodeCheckXXX functions (#4106) FindNodeCheck is not clear about what the function is doing. They are renamed to FindNodeMatchingCheckFunctionXXX. Also for choosing elements in these functions, CheckNodeFunc type is introduced. --- .../planner/insert_select_planner.c | 2 +- .../planner/multi_logical_planner.c | 41 +++++++++++-------- .../planner/multi_router_planner.c | 18 +++++--- .../planner/query_colocation_checker.c | 3 +- .../planner/query_pushdown_planning.c | 19 +++++---- .../distributed/planner/recursive_planning.c | 11 ++--- src/backend/distributed/utils/citus_clauses.c | 4 +- .../distributed/multi_logical_planner.h | 5 ++- 8 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/backend/distributed/planner/insert_select_planner.c b/src/backend/distributed/planner/insert_select_planner.c index 6f25b1a59..aa05f9e0d 100644 --- a/src/backend/distributed/planner/insert_select_planner.c +++ b/src/backend/distributed/planner/insert_select_planner.c @@ -570,7 +570,7 @@ DistributedInsertSelectSupported(Query *queryTree, RangeTblEntry *insertRte, } } - if (FindNodeCheck((Node *) queryTree, CitusIsVolatileFunction)) + if (FindNodeMatchingCheckFunction((Node *) queryTree, CitusIsVolatileFunction)) { return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, "volatile functions are not allowed in distributed " diff --git a/src/backend/distributed/planner/multi_logical_planner.c b/src/backend/distributed/planner/multi_logical_planner.c index f2c046597..6e51e779c 100644 --- a/src/backend/distributed/planner/multi_logical_planner.c +++ b/src/backend/distributed/planner/multi_logical_planner.c @@ -70,6 +70,8 @@ typedef MultiNode *(*RuleApplyFunction) (MultiNode *leftNode, MultiNode *rightNo List *partitionColumnList, JoinType joinType, List *joinClauses); +typedef bool (*CheckNodeFunc)(Node *); + static RuleApplyFunction RuleApplyFunctionArray[JOIN_RULE_LAST] = { 0 }; /* join rules */ /* Local functions forward declarations */ @@ -170,20 +172,20 @@ MultiLogicalPlanCreate(Query *originalQuery, Query *queryTree, /* - * FindNodeCheck finds a node for which the check function returns true. + * FindNodeMatchingCheckFunction finds a node for which the checker function returns true. * * To call this function directly with an RTE, use: - * range_table_walker(rte, FindNodeCheck, check, QTW_EXAMINE_RTES_BEFORE) + * range_table_walker(rte, FindNodeMatchingCheckFunction, checker, QTW_EXAMINE_RTES_BEFORE) */ bool -FindNodeCheck(Node *node, bool (*check)(Node *)) +FindNodeMatchingCheckFunction(Node *node, CheckNodeFunc checker) { if (node == NULL) { return false; } - if (check(node)) + if (checker(node)) { return true; } @@ -195,11 +197,11 @@ FindNodeCheck(Node *node, bool (*check)(Node *)) } else if (IsA(node, Query)) { - return query_tree_walker((Query *) node, FindNodeCheck, check, + return query_tree_walker((Query *) node, FindNodeMatchingCheckFunction, checker, QTW_EXAMINE_RTES_BEFORE); } - return expression_tree_walker(node, FindNodeCheck, check); + return expression_tree_walker(node, FindNodeMatchingCheckFunction, checker); } @@ -267,7 +269,8 @@ TargetListOnPartitionColumn(Query *query, List *targetEntryList) */ if (!targetListOnPartitionColumn) { - if (!FindNodeCheckInRangeTableList(query->rtable, IsDistributedTableRTE) && + if (!FindNodeMatchingCheckFunctionInRangeTableList(query->rtable, + IsDistributedTableRTE) && AllTargetExpressionsAreColumnReferences(targetEntryList)) { targetListOnPartitionColumn = true; @@ -332,16 +335,17 @@ AllTargetExpressionsAreColumnReferences(List *targetEntryList) /* - * FindNodeCheckInRangeTableList finds a node for which the check + * FindNodeMatchingCheckFunctionInRangeTableList finds a node for which the checker * function returns true. * - * FindNodeCheckInRangeTableList relies on FindNodeCheck() but only - * considers the range table entries. + * FindNodeMatchingCheckFunctionInRangeTableList relies on + * FindNodeMatchingCheckFunction() but only considers the range table entries. */ bool -FindNodeCheckInRangeTableList(List *rtable, bool (*check)(Node *)) +FindNodeMatchingCheckFunctionInRangeTableList(List *rtable, CheckNodeFunc checker) { - return range_table_walker(rtable, FindNodeCheck, check, QTW_EXAMINE_RTES_BEFORE); + return range_table_walker(rtable, FindNodeMatchingCheckFunction, checker, + QTW_EXAMINE_RTES_BEFORE); } @@ -737,7 +741,7 @@ MultiNodeTree(Query *queryTree) bool ContainsReadIntermediateResultFunction(Node *node) { - return FindNodeCheck(node, IsReadIntermediateResultFunction); + return FindNodeMatchingCheckFunction(node, IsReadIntermediateResultFunction); } @@ -749,7 +753,7 @@ ContainsReadIntermediateResultFunction(Node *node) bool ContainsReadIntermediateResultArrayFunction(Node *node) { - return FindNodeCheck(node, IsReadIntermediateResultArrayFunction); + return FindNodeMatchingCheckFunction(node, IsReadIntermediateResultArrayFunction); } @@ -793,7 +797,8 @@ IsCitusExtraDataContainerRelation(RangeTblEntry *rte) return false; } - return FindNodeCheck((Node *) rte->functions, IsCitusExtraDataContainerFunc); + return FindNodeMatchingCheckFunction((Node *) rte->functions, + IsCitusExtraDataContainerFunc); } @@ -933,7 +938,7 @@ DeferErrorIfQueryNotSupported(Query *queryTree) errorHint = filterHint; } - if (FindNodeCheck((Node *) queryTree, IsGroupingFunc)) + if (FindNodeMatchingCheckFunction((Node *) queryTree, IsGroupingFunc)) { preconditionsSatisfied = false; errorMessage = "could not run distributed query with GROUPING"; @@ -966,13 +971,13 @@ DeferErrorIfQueryNotSupported(Query *queryTree) errorHint = filterHint; } - if (FindNodeCheck((Node *) queryTree->limitCount, IsNodeSubquery)) + if (FindNodeMatchingCheckFunction((Node *) queryTree->limitCount, IsNodeSubquery)) { preconditionsSatisfied = false; errorMessage = "subquery in LIMIT is not supported in multi-shard queries"; } - if (FindNodeCheck((Node *) queryTree->limitOffset, IsNodeSubquery)) + if (FindNodeMatchingCheckFunction((Node *) queryTree->limitOffset, IsNodeSubquery)) { preconditionsSatisfied = false; errorMessage = "subquery in OFFSET is not supported in multi-shard queries"; diff --git a/src/backend/distributed/planner/multi_router_planner.c b/src/backend/distributed/planner/multi_router_planner.c index e322622bc..d6d7dd3f0 100644 --- a/src/backend/distributed/planner/multi_router_planner.c +++ b/src/backend/distributed/planner/multi_router_planner.c @@ -648,7 +648,8 @@ ModifyPartialQuerySupported(Query *queryTree, bool multiShardQuery, if (cteQuery->hasForUpdate && - FindNodeCheckInRangeTableList(cteQuery->rtable, IsReferenceTableRTE)) + FindNodeMatchingCheckFunctionInRangeTableList(cteQuery->rtable, + IsReferenceTableRTE)) { return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, "Router planner doesn't support SELECT FOR UPDATE" @@ -656,7 +657,7 @@ ModifyPartialQuerySupported(Query *queryTree, bool multiShardQuery, NULL, NULL); } - if (FindNodeCheck((Node *) cteQuery, CitusIsVolatileFunction)) + if (FindNodeMatchingCheckFunction((Node *) cteQuery, CitusIsVolatileFunction)) { return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, "Router planner doesn't support VOLATILE functions" @@ -705,7 +706,8 @@ ModifyPartialQuerySupported(Query *queryTree, bool multiShardQuery, } if (commandType == CMD_UPDATE && - FindNodeCheck((Node *) targetEntry->expr, CitusIsVolatileFunction)) + FindNodeMatchingCheckFunction((Node *) targetEntry->expr, + CitusIsVolatileFunction)) { return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, "functions used in UPDATE queries on distributed " @@ -733,7 +735,8 @@ ModifyPartialQuerySupported(Query *queryTree, bool multiShardQuery, if (joinTree != NULL) { - if (FindNodeCheck((Node *) joinTree->quals, CitusIsVolatileFunction)) + if (FindNodeMatchingCheckFunction((Node *) joinTree->quals, + CitusIsVolatileFunction)) { return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, "functions used in the WHERE clause of modification " @@ -830,7 +833,9 @@ ModifyQuerySupported(Query *queryTree, Query *originalQuery, bool multiShardQuer if (!fastPathRouterQuery && ContainsReadIntermediateResultFunction((Node *) originalQuery)) { - bool hasTidColumn = FindNodeCheck((Node *) originalQuery->jointree, IsTidColumn); + bool hasTidColumn = FindNodeMatchingCheckFunction( + (Node *) originalQuery->jointree, IsTidColumn); + if (hasTidColumn) { return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, @@ -1138,7 +1143,8 @@ MultiShardModifyQuerySupported(Query *originalQuery, "ON instead", NULL, NULL); } - else if (FindNodeCheck((Node *) originalQuery, CitusIsVolatileFunction)) + else if (FindNodeMatchingCheckFunction((Node *) originalQuery, + CitusIsVolatileFunction)) { errorMessage = DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, "functions used in UPDATE queries on distributed " diff --git a/src/backend/distributed/planner/query_colocation_checker.c b/src/backend/distributed/planner/query_colocation_checker.c index 407705820..60300535f 100644 --- a/src/backend/distributed/planner/query_colocation_checker.c +++ b/src/backend/distributed/planner/query_colocation_checker.c @@ -141,7 +141,8 @@ AnchorRte(Query *subquery) * the set operations. */ if (anchorRangeTblEntry == NULL && currentRte->rtekind == RTE_SUBQUERY && - FindNodeCheck((Node *) currentRte->subquery, IsDistributedTableRTE) && + FindNodeMatchingCheckFunction((Node *) currentRte->subquery, + IsDistributedTableRTE) && currentRte->subquery->setOperations == NULL && !ContainsUnionSubquery(currentRte->subquery)) { diff --git a/src/backend/distributed/planner/query_pushdown_planning.c b/src/backend/distributed/planner/query_pushdown_planning.c index 4296d9272..8b57b9682 100644 --- a/src/backend/distributed/planner/query_pushdown_planning.c +++ b/src/backend/distributed/planner/query_pushdown_planning.c @@ -150,7 +150,7 @@ ShouldUseSubqueryPushDown(Query *originalQuery, Query *rewrittenQuery, * We process function RTEs as subqueries, since the join order planner * does not know how to handle them. */ - if (FindNodeCheck((Node *) originalQuery, IsFunctionRTE)) + if (FindNodeMatchingCheckFunction((Node *) originalQuery, IsFunctionRTE)) { return true; } @@ -159,7 +159,7 @@ ShouldUseSubqueryPushDown(Query *originalQuery, Query *rewrittenQuery, * We handle outer joins as subqueries, since the join order planner * does not know how to handle them. */ - if (FindNodeCheck((Node *) originalQuery->jointree, IsOuterJoinExpr)) + if (FindNodeMatchingCheckFunction((Node *) originalQuery->jointree, IsOuterJoinExpr)) { return true; } @@ -170,7 +170,7 @@ ShouldUseSubqueryPushDown(Query *originalQuery, Query *rewrittenQuery, * An example of this is https://github.com/citusdata/citus/issues/2739 * where postgres pulls-up the outer-join in the subquery. */ - if (FindNodeCheck((Node *) rewrittenQuery->jointree, IsOuterJoinExpr)) + if (FindNodeMatchingCheckFunction((Node *) rewrittenQuery->jointree, IsOuterJoinExpr)) { return true; } @@ -278,7 +278,7 @@ JoinTreeContainsSubqueryWalker(Node *joinTreeNode, void *context) bool WhereOrHavingClauseContainsSubquery(Query *query) { - if (FindNodeCheck(query->havingQual, IsNodeSubquery)) + if (FindNodeMatchingCheckFunction(query->havingQual, IsNodeSubquery)) { return true; } @@ -294,7 +294,7 @@ WhereOrHavingClauseContainsSubquery(Query *query) * JoinExpr nodes that also have quals. If that's the case we need to check * those as well if they contain andy subqueries. */ - return FindNodeCheck((Node *) query->jointree, IsNodeSubquery); + return FindNodeMatchingCheckFunction((Node *) query->jointree, IsNodeSubquery); } @@ -305,7 +305,7 @@ WhereOrHavingClauseContainsSubquery(Query *query) bool TargetListContainsSubquery(Query *query) { - return FindNodeCheck((Node *) query->targetList, IsNodeSubquery); + return FindNodeMatchingCheckFunction((Node *) query->targetList, IsNodeSubquery); } @@ -724,7 +724,8 @@ FromClauseRecurringTupleType(Query *queryTree) return RECURRING_TUPLES_EMPTY_JOIN_TREE; } - if (FindNodeCheckInRangeTableList(queryTree->rtable, IsDistributedTableRTE)) + if (FindNodeMatchingCheckFunctionInRangeTableList(queryTree->rtable, + IsDistributedTableRTE)) { /* * There is a distributed table somewhere in the FROM clause. @@ -1337,8 +1338,8 @@ RelationInfoContainsOnlyRecurringTuples(PlannerInfo *plannerInfo, { RangeTblEntry *rangeTableEntry = plannerInfo->simple_rte_array[relationId]; - if (FindNodeCheckInRangeTableList(list_make1(rangeTableEntry), - IsDistributedTableRTE)) + if (FindNodeMatchingCheckFunctionInRangeTableList(list_make1(rangeTableEntry), + IsDistributedTableRTE)) { /* we already found a distributed table, no need to check further */ return false; diff --git a/src/backend/distributed/planner/recursive_planning.c b/src/backend/distributed/planner/recursive_planning.c index 253ec2aaf..ad337115f 100644 --- a/src/backend/distributed/planner/recursive_planning.c +++ b/src/backend/distributed/planner/recursive_planning.c @@ -376,7 +376,7 @@ ShouldRecursivelyPlanNonColocatedSubqueries(Query *subquery, } /* direct joins with local tables are not supported by any of Citus planners */ - if (FindNodeCheckInRangeTableList(subquery->rtable, IsLocalTableRTE)) + if (FindNodeMatchingCheckFunctionInRangeTableList(subquery->rtable, IsLocalTableRTE)) { return false; } @@ -636,7 +636,8 @@ ShouldRecursivelyPlanAllSubqueriesInWhere(Query *query) return false; } - if (FindNodeCheckInRangeTableList(query->rtable, IsDistributedTableRTE)) + if (FindNodeMatchingCheckFunctionInRangeTableList(query->rtable, + IsDistributedTableRTE)) { /* there is a distributed table in the FROM clause */ return false; @@ -662,7 +663,7 @@ RecursivelyPlanAllSubqueries(Node *node, RecursivePlanningContext *planningConte if (IsA(node, Query)) { Query *query = (Query *) node; - if (FindNodeCheckInRangeTableList(query->rtable, IsCitusTableRTE)) + if (FindNodeMatchingCheckFunctionInRangeTableList(query->rtable, IsCitusTableRTE)) { RecursivelyPlanSubquery(query, planningContext); } @@ -876,7 +877,7 @@ RecursivelyPlanSubqueryWalker(Node *node, RecursivePlanningContext *context) static bool ShouldRecursivelyPlanSubquery(Query *subquery, RecursivePlanningContext *context) { - if (FindNodeCheckInRangeTableList(subquery->rtable, IsLocalTableRTE)) + if (FindNodeMatchingCheckFunctionInRangeTableList(subquery->rtable, IsLocalTableRTE)) { /* * Postgres can always plan queries that don't require distributed planning. @@ -1029,7 +1030,7 @@ RecursivelyPlanSetOperations(Query *query, Node *node, Query *subquery = rangeTableEntry->subquery; if (rangeTableEntry->rtekind == RTE_SUBQUERY && - FindNodeCheck((Node *) subquery, IsDistributedTableRTE)) + FindNodeMatchingCheckFunction((Node *) subquery, IsDistributedTableRTE)) { RecursivelyPlanSubquery(subquery, context); } diff --git a/src/backend/distributed/utils/citus_clauses.c b/src/backend/distributed/utils/citus_clauses.c index 7a9e6e72a..7700364cd 100644 --- a/src/backend/distributed/utils/citus_clauses.c +++ b/src/backend/distributed/utils/citus_clauses.c @@ -57,7 +57,7 @@ RequiresCoordinatorEvaluation(Query *query) return false; } - return FindNodeCheck((Node *) query, CitusIsMutableFunction); + return FindNodeMatchingCheckFunction((Node *) query, CitusIsMutableFunction); } @@ -152,7 +152,7 @@ PartiallyEvaluateExpression(Node *expression, coordinatorEvaluationContext); } - if (FindNodeCheck(expression, IsVariableExpression)) + if (FindNodeMatchingCheckFunction(expression, IsVariableExpression)) { /* * The expression contains a variable expression (e.g. a stable function, diff --git a/src/include/distributed/multi_logical_planner.h b/src/include/distributed/multi_logical_planner.h index ae6224d47..ba6845958 100644 --- a/src/include/distributed/multi_logical_planner.h +++ b/src/include/distributed/multi_logical_planner.h @@ -187,9 +187,10 @@ typedef struct MultiExtendedOp extern MultiTreeRoot * MultiLogicalPlanCreate(Query *originalQuery, Query *queryTree, PlannerRestrictionContext * plannerRestrictionContext); -extern bool FindNodeCheck(Node *node, bool (*check)(Node *)); +extern bool FindNodeMatchingCheckFunction(Node *node, bool (*check)(Node *)); extern bool TargetListOnPartitionColumn(Query *query, List *targetEntryList); -extern bool FindNodeCheckInRangeTableList(List *rtable, bool (*check)(Node *)); +extern bool FindNodeMatchingCheckFunctionInRangeTableList(List *rtable, bool (*check)( + Node *)); extern bool IsCitusTableRTE(Node *node); extern bool IsDistributedTableRTE(Node *node); extern bool IsReferenceTableRTE(Node *node);