Apply code-review feedback

src/backend/distributed/planner/multi_logical_planner.c

@@ -103,7 +103,6 @@ static MultiNode * MultiJoinTree(List *joinOrderList, List *collectTableList,
 static MultiCollect * CollectNodeForTable(List *collectTableList, uint32 rangeTableId);
 static MultiSelect * MultiSelectNode(List *whereClauseList);
 static bool IsSelectClause(Node *clause);
-static void ErrorIfSublink(Node *clause);
 static bool IsSublinkClause(Node *clause);
 static MultiProject * MultiProjectNode(List *targetEntryList);
 static MultiExtendedOp * MultiExtendedOpNode(Query *queryTree);
@@ -195,7 +194,7 @@ MultiLogicalPlanCreate(Query *originalQuery, Query *queryTree,
 
 /*
  * SublinkList finds the subquery nodes in the where clause of the given query. Note
- * that the function should be called on the orignal query given that postgres
+ * that the function should be called on the original query given that postgres
  * standard_planner() may convert the subqueries in WHERE clause to joins.
  */
 static List *
@@ -436,8 +435,9 @@ DeferErrorIfUnsupportedSubqueryPushdown(Query *originalQuery,
 	}
 
 	/*
-	 * We first extract all the queries that appear in the orignal query. Later,
+	 * We first extract all the queries that appear in the original query. Later,
 	 * we delete the original query given that error rules does not apply to the
+	 * top level query. For instance, we could support any LIMIT/ORDER BY on the
 	 * top level query.
 	 */
 	ExtractQueryWalker((Node *) originalQuery, &subqueryList);
@@ -589,13 +589,11 @@ EqualOpExpressionLists(List *firstOpExpressionList, List *secondOpExpressionList
 
 
 /*
- * DeferErrorIfCannotPushdownSubquery recursively checks if we can push down the given
+ * DeferErrorIfCannotPushdownSubquery checks if we can push down the given
  * subquery to worker nodes. If we cannot push down the subquery, this function
  * returns a deferred error.
  *
- * We can push down a subquery if it follows rules below. We support nested queries
+ * We can push down a subquery if it follows rules below:
- * as long as they follow the same rules, and we recurse to validate each subquery
- * for this given query.
  * a. If there is an aggregate, it must be grouped on partition column.
  * b. If there is a join, it must be between two regular tables or two subqueries.
  * We don't support join between a regular table and a subquery. And columns on
@@ -1279,6 +1277,10 @@ ErrorIfQueryNotSupported(Query *queryTree)
 	const char *filterHint = "Consider using an equality filter on the distributed "
 							 "table's partition column.";
 
+	/*
+	 * There could be Sublinks in the target list as well. To produce better
+	 * error messages we're checking sublinks in the where clause.
+	 */
 	if (queryTree->hasSubLinks && SublinkList(queryTree) == NIL)
 	{
 		preconditionsSatisfied = false;
@@ -1789,8 +1791,11 @@ ValidateClauseList(List *clauseList)
 	{
 		Node *clause = (Node *) lfirst(clauseCell);
 
-		/* produce a better error message for sublinks */
+		/*
-		ErrorIfSublink(clause);
+		 * There could never be sublinks here given that it is handled
+		 * in subquery pushdown code-path.
+		 */
+		Assert(!IsSublinkClause(clause));
 
 		if (!(IsSelectClause(clause) || IsJoinClause(clause) || or_clause(clause)))
 		{
@@ -1808,7 +1813,7 @@ ValidateClauseList(List *clauseList)
  * prevents erroneous results.
  *
  * Note that this function is slightly different than ValidateClauseList(),
- * additionally allowing subkinks.
+ * additionally allowing sublinks.
  */
 static void
 ValidateSubqueryPushdownClauseList(List *clauseList)
@@ -2307,25 +2312,6 @@ IsSelectClause(Node *clause)
 }
 
 
-/*
- * ErrorIfSublink errors out if the input claise is either sublink or subplan.
- */
-static void
-ErrorIfSublink(Node *clause)
-{
-	NodeTag nodeTag = nodeTag(clause);
-
-	/* error out for subqueries in WHERE clause */
-	if (nodeTag == T_SubLink || nodeTag == T_SubPlan)
-	{
-		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						errmsg("cannot perform distributed planning on this query"),
-						errdetail("Subqueries other than in from-clause are currently "
-								  "unsupported")));
-	}
-}
-
-
 /*
  * IsSublinkClause determines if the given node is a sublink or subplan.
  */

src/backend/distributed/planner/multi_router_planner.c

@@ -3038,6 +3038,8 @@ CopyRelationRestrictionContext(RelationRestrictionContext *oldContext)
 
 		/* not copyable, but readonly */
 		newRestriction->plannerInfo = oldRestriction->plannerInfo;
+		newRestriction->prunedShardIntervalList =
+			copyObject(oldRestriction->prunedShardIntervalList);
 
 		newContext->relationRestrictionList =
 			lappend(newContext->relationRestrictionList, newRestriction);

src/test/regress/expected/multi_insert_select.out

@@ -121,16 +121,6 @@ FROM
 WHERE
   user_id < 0;
 NOTICE:  evaluating on master
--- make sure stable functions in CTEs are evaluated
-INSERT INTO raw_events_second (user_id, value_1)
-WITH sub_cte AS (SELECT evaluate_on_master())
-SELECT
-  user_id, (SELECT * FROM sub_cte)
-FROM
-  raw_events_first
-WHERE
-  user_id < 0;
-ERROR:  Subqueries without relations are not allowed in INSERT ... SELECT queries
 -- make sure we don't evaluate stable functions with column arguments
 CREATE OR REPLACE FUNCTION evaluate_on_master(x int)
 RETURNS int LANGUAGE plpgsql STABLE
@@ -665,7 +655,7 @@ INSERT INTO agg_events
               fist_table_agg;
 ERROR:  INSERT INTO ... SELECT partition columns in the source table and subquery do not match
 DETAIL:  The target table's partition column should correspond to a partition column in the subquery.
--- We do support some CTEs
+-- We don't support CTEs that consist of const values as well
 INSERT INTO agg_events
   WITH sub_cte AS (SELECT 1)
   SELECT

src/test/regress/expected/multi_subquery_in_where_clause.out

@@ -4,8 +4,6 @@
 -- the tables that are used depends to multi_insert_select_behavioral_analytics_create_table.sql
 --
 -- We don't need shard id sequence here, so commented out to prevent conflicts with concurrent tests
--- ALTER SEQUENCE pg_catalog.pg_dist_shardid_seq RESTART 1430000;
--- ALTER SEQUENCE pg_catalog.pg_dist_jobid_seq RESTART 1430000;
 -- subqueries in WHERE with greater operator
 SELECT 
   user_id
@@ -150,7 +148,7 @@ ORDER BY
       25
 (4 rows)
 
--- the following query doesn't have a meaningul result
+-- the following query doesn't have a meaningful result
 -- but it is a valid query with an arbitrary subquery in
 -- WHERE clause
 SELECT user_id, value_2 FROM users_table WHERE
@@ -225,7 +223,7 @@ ORDER BY 1, 2;
 (7 rows)
 
 -- similar to the above query
--- the following query doesn't have a meaningul result
+-- the following query doesn't have a meaningful result
 -- but it is a valid query with an arbitrary subquery in
 -- WHERE clause
 SELECT 
@@ -244,7 +242,7 @@ WHERE
 	  (
 	    (SELECT
 	      users_table.user_id,
-	      'action=>1'AS event,
+	      'action=>1' AS event,
 	      events_table.time
 	    FROM
 	      users_table,
@@ -258,7 +256,7 @@ WHERE
 	    UNION
 	    (SELECT
 	      users_table.user_id,
-	      'action=>2'AS event,
+	      'action=>2' AS event,
 	      events_table.time
 	    FROM
 	      users_table,
@@ -305,7 +303,7 @@ ORDER BY 1;
       69
 (6 rows)
 
--- the following query doesn't have a meaningul result
+-- the following query doesn't have a meaningful result
 -- but it is a valid query with an arbitrary subquery in
 -- FROM clause involving a complex query in WHERE clause
 SELECT user_id, array_length(events_table, 1)
@@ -421,7 +419,7 @@ WHERE
 	  (
 	    (SELECT
 	      2 * users_table.user_id as user_id,
-	      'action=>1'AS event,
+	      'action=>1' AS event,
 	      events_table.time
 	    FROM
 	      users_table,
@@ -435,7 +433,7 @@ WHERE
 	    UNION
 	    (SELECT
 	      users_table.user_id,
-	      'action=>2'AS event,
+	      'action=>2' AS event,
 	      events_table.time
 	    FROM
 	      users_table,
@@ -530,8 +528,8 @@ WHERE
           );
 ERROR:  cannot push down this subquery
 DETAIL:  Offset clause is currently unsupported
--- we can detect errors even if they appear in WHERE -> FROM -> WHERE
+-- we can detect unsupported subquerues even if they appear
--- subqueries
+-- in WHERE subquery -> FROM subquery -> WHERE subquery
 SELECT user_id 
 FROM   users_table 
 WHERE  user_id 

src/test/regress/sql/multi_insert_select.sql

@@ -96,16 +96,6 @@ FROM
 WHERE
   user_id < 0;
 
--- make sure stable functions in CTEs are evaluated
-INSERT INTO raw_events_second (user_id, value_1)
-WITH sub_cte AS (SELECT evaluate_on_master())
-SELECT
-  user_id, (SELECT * FROM sub_cte)
-FROM
-  raw_events_first
-WHERE
-  user_id < 0;
-
 -- make sure we don't evaluate stable functions with column arguments
 CREATE OR REPLACE FUNCTION evaluate_on_master(x int)
 RETURNS int LANGUAGE plpgsql STABLE
@@ -517,7 +507,7 @@ INSERT INTO agg_events
             FROM
               fist_table_agg;
 
--- We do support some CTEs
+-- We don't support CTEs that consist of const values as well
 INSERT INTO agg_events
   WITH sub_cte AS (SELECT 1)
   SELECT

src/test/regress/sql/multi_subquery_in_where_clause.sql

@@ -4,8 +4,6 @@
 -- the tables that are used depends to multi_insert_select_behavioral_analytics_create_table.sql
 --
 -- We don't need shard id sequence here, so commented out to prevent conflicts with concurrent tests
--- ALTER SEQUENCE pg_catalog.pg_dist_shardid_seq RESTART 1430000;
--- ALTER SEQUENCE pg_catalog.pg_dist_jobid_seq RESTART 1430000;
 
 -- subqueries in WHERE with greater operator
 SELECT 
@@ -107,7 +105,7 @@ ORDER BY
 	user_id;
 
 
--- the following query doesn't have a meaningul result
+-- the following query doesn't have a meaningful result
 -- but it is a valid query with an arbitrary subquery in
 -- WHERE clause
 SELECT user_id, value_2 FROM users_table WHERE
@@ -173,7 +171,7 @@ ORDER BY 1, 2;
 
 
 -- similar to the above query
--- the following query doesn't have a meaningul result
+-- the following query doesn't have a meaningful result
 -- but it is a valid query with an arbitrary subquery in
 -- WHERE clause
 SELECT 
@@ -192,7 +190,7 @@ WHERE
 	  (
 	    (SELECT
 	      users_table.user_id,
-	      'action=>1'AS event,
+	      'action=>1' AS event,
 	      events_table.time
 	    FROM
 	      users_table,
@@ -206,7 +204,7 @@ WHERE
 	    UNION
 	    (SELECT
 	      users_table.user_id,
-	      'action=>2'AS event,
+	      'action=>2' AS event,
 	      events_table.time
 	    FROM
 	      users_table,
@@ -245,7 +243,7 @@ HAVING count(*) > 3 AND sum(value_2) > 49000
 ORDER BY 1;
 
 
--- the following query doesn't have a meaningul result
+-- the following query doesn't have a meaningful result
 -- but it is a valid query with an arbitrary subquery in
 -- FROM clause involving a complex query in WHERE clause
 SELECT user_id, array_length(events_table, 1)
@@ -357,7 +355,7 @@ WHERE
 	  (
 	    (SELECT
 	      2 * users_table.user_id as user_id,
-	      'action=>1'AS event,
+	      'action=>1' AS event,
 	      events_table.time
 	    FROM
 	      users_table,
@@ -371,7 +369,7 @@ WHERE
 	    UNION
 	    (SELECT
 	      users_table.user_id,
-	      'action=>2'AS event,
+	      'action=>2' AS event,
 	      events_table.time
 	    FROM
 	      users_table,
@@ -462,8 +460,8 @@ WHERE
            OFFSET 3
           );
 
--- we can detect errors even if they appear in WHERE -> FROM -> WHERE
+-- we can detect unsupported subquerues even if they appear
--- subqueries
+-- in WHERE subquery -> FROM subquery -> WHERE subquery
 SELECT user_id 
 FROM   users_table 
 WHERE  user_id