From 30f10984e142dafb9617f042ed61f4a7a10e4154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Fri, 31 Jan 2020 21:46:54 +0000 Subject: [PATCH] Defer get_agg_clause_costs, it happens later & avoids errors --- .../planner/multi_logical_optimizer.c | 15 ------------- .../planner/multi_logical_planner.c | 11 ---------- .../planner/query_colocation_checker.c | 4 ++-- .../distributed/multi_logical_planner.h | 1 - src/test/regress/expected/multi_subquery.out | 21 +++++++++++++++---- src/test/regress/sql/multi_subquery.sql | 2 +- 6 files changed, 20 insertions(+), 34 deletions(-) diff --git a/src/backend/distributed/planner/multi_logical_optimizer.c b/src/backend/distributed/planner/multi_logical_optimizer.c index 9dd0b12a6..1343248c0 100644 --- a/src/backend/distributed/planner/multi_logical_optimizer.c +++ b/src/backend/distributed/planner/multi_logical_optimizer.c @@ -1588,7 +1588,6 @@ MasterAggregateExpression(Aggref *originalAggregate, const AttrNumber argumentId = 1; /* our aggregates have single arguments */ AggregateType aggregateType = GetAggregateType(originalAggregate); Expr *newMasterExpression = NULL; - AggClauseCosts aggregateCosts; if (walkerContext->extendedOpNodeProperties->pullUpIntermediateRows) { @@ -2074,12 +2073,6 @@ MasterAggregateExpression(Aggref *originalAggregate, newMasterExpression = typeConvertedExpression; } - /* Run AggRefs through cost machinery to mark required fields sanely */ - memset(&aggregateCosts, 0, sizeof(aggregateCosts)); - - get_agg_clause_costs(NULL, (Node *) newMasterExpression, AGGSPLIT_SIMPLE, - &aggregateCosts); - return newMasterExpression; } @@ -2968,7 +2961,6 @@ WorkerAggregateExpressionList(Aggref *originalAggregate, } AggregateType aggregateType = GetAggregateType(originalAggregate); - AggClauseCosts aggregateCosts; if (aggregateType == AGGREGATE_COUNT && originalAggregate->aggdistinct && CountDistinctErrorRate == DISABLE_DISTINCT_APPROXIMATION && @@ -3145,13 +3137,6 @@ WorkerAggregateExpressionList(Aggref *originalAggregate, workerAggregateList = lappend(workerAggregateList, workerAggregate); } - - /* Run AggRefs through cost machinery to mark required fields sanely */ - memset(&aggregateCosts, 0, sizeof(aggregateCosts)); - - get_agg_clause_costs(NULL, (Node *) workerAggregateList, AGGSPLIT_SIMPLE, - &aggregateCosts); - return workerAggregateList; } diff --git a/src/backend/distributed/planner/multi_logical_planner.c b/src/backend/distributed/planner/multi_logical_planner.c index f6397a4aa..549900a5e 100644 --- a/src/backend/distributed/planner/multi_logical_planner.c +++ b/src/backend/distributed/planner/multi_logical_planner.c @@ -404,17 +404,6 @@ FindNodeCheckInRangeTableList(List *rtable, bool (*check)(Node *)) } -/* - * QueryContainsDistributedTableRTE determines whether the given - * query contains a distributed table. - */ -bool -QueryContainsDistributedTableRTE(Query *query) -{ - return FindNodeCheck((Node *) query, IsDistributedTableRTE); -} - - /* * NodeTryGetRteRelid returns the relid of the given RTE_RELATION RangeTableEntry. * Returns InvalidOid if any of these assumptions fail for given node. diff --git a/src/backend/distributed/planner/query_colocation_checker.c b/src/backend/distributed/planner/query_colocation_checker.c index a520a9cfd..ed386c7cd 100644 --- a/src/backend/distributed/planner/query_colocation_checker.c +++ b/src/backend/distributed/planner/query_colocation_checker.c @@ -141,7 +141,7 @@ AnchorRte(Query *subquery) * the set operations. */ if (anchorRangeTblEntry == NULL && currentRte->rtekind == RTE_SUBQUERY && - QueryContainsDistributedTableRTE(currentRte->subquery) && + FindNodeCheck((Node *) currentRte->subquery, IsDistributedTableRTE) && currentRte->subquery->setOperations == NULL && !ContainsUnionSubquery(currentRte->subquery)) { @@ -195,7 +195,7 @@ SubqueryColocated(Query *subquery, ColocatedJoinChecker *checker) */ if (list_length(filteredRestrictionList) == 0) { - Assert(!QueryContainsDistributedTableRTE(subquery)); + Assert(!FindNodeCheck((Node *) subquery, IsDistributedTableRTE)); return true; } diff --git a/src/include/distributed/multi_logical_planner.h b/src/include/distributed/multi_logical_planner.h index 4cafe1a40..a5ec693f6 100644 --- a/src/include/distributed/multi_logical_planner.h +++ b/src/include/distributed/multi_logical_planner.h @@ -193,7 +193,6 @@ extern bool TargetListOnPartitionColumn(Query *query, List *targetEntryList); extern bool FindNodeCheckInRangeTableList(List *rtable, bool (*check)(Node *)); extern bool IsCitusTableRTE(Node *node); extern bool IsDistributedTableRTE(Node *node); -extern bool QueryContainsDistributedTableRTE(Query *query); extern bool IsCitusExtraDataContainerRelation(RangeTblEntry *rte); extern bool ContainsReadIntermediateResultFunction(Node *node); extern bool ContainsReadIntermediateResultArrayFunction(Node *node); diff --git a/src/test/regress/expected/multi_subquery.out b/src/test/regress/expected/multi_subquery.out index c5b41d5f3..ba2f0c845 100644 --- a/src/test/regress/expected/multi_subquery.out +++ b/src/test/regress/expected/multi_subquery.out @@ -898,15 +898,28 @@ ERROR: Subqueries in HAVING cannot refer to outer query SELECT t1.event_type FROM events_table t1 GROUP BY t1.event_type HAVING t1.event_type > avg((SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) ORDER BY 1; -ERROR: cannot handle unplanned sub-select + event_type +--------------------------------------------------------------------- + 6 +(1 row) + SELECT t1.event_type FROM events_table t1 GROUP BY t1.event_type HAVING t1.event_type > avg(2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) ORDER BY 1; -ERROR: cannot handle unplanned sub-select + event_type +--------------------------------------------------------------------- +(0 rows) + SELECT t1.event_type FROM events_table t1 -GROUP BY t1.event_type HAVING t1.event_type > avg(t1.value_2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) +GROUP BY t1.event_type HAVING t1.event_type > avg((SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1) - t1.value_2) ORDER BY 1; -ERROR: cannot handle unplanned sub-select + event_type +--------------------------------------------------------------------- + 4 + 5 + 6 +(3 rows) + RESET citus.coordinator_aggregation_strategy; SELECT t1.event_type FROM events_table t1 GROUP BY t1.event_type HAVING t1.event_type > corr(t1.value_3, t1.value_2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) diff --git a/src/test/regress/sql/multi_subquery.sql b/src/test/regress/sql/multi_subquery.sql index cf4c47a93..eb47ce0d0 100644 --- a/src/test/regress/sql/multi_subquery.sql +++ b/src/test/regress/sql/multi_subquery.sql @@ -642,7 +642,7 @@ GROUP BY t1.event_type HAVING t1.event_type > avg(2 + (SELECT t2.value_2 FROM us ORDER BY 1; SELECT t1.event_type FROM events_table t1 -GROUP BY t1.event_type HAVING t1.event_type > avg(t1.value_2 + (SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1)) +GROUP BY t1.event_type HAVING t1.event_type > avg((SELECT t2.value_2 FROM users_table t2 ORDER BY 1 DESC LIMIT 1) - t1.value_2) ORDER BY 1; RESET citus.coordinator_aggregation_strategy;