Prevent generation of bogus subquery scan paths.

Commit 0927d2f46ddd4cf7d6bf2cc84b3be923e0aedc52 didn't check that
consider_parallel was set for the target relation or account for
the possibility that required_outer might be non-empty.

To prevent future bugs of this ilk, add some assertions to
add_partial_path and do a bit of future-proofing of the code
recently added to recurse_set_operations.

Report by Andreas Seltenreich.  Patch by Jeevan Chalke.  Review
by Amit Kapila and by me.

Discussion: http://postgr.es/m/CAM2+6=U+9otsyF2fYB8x_2TBeHTR90itarqW=qAEjN-kHaC7kw@mail.gmail.com

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 9ed73da0f79c6d3885921502b3aed802e2beba35..afc663cfd8ffcc5e6d4c84e5e71e7da08f94a2d3 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2243,26 +2243,31 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
                                                                                  pathkeys, required_outer));
        }
 
-       /* If consider_parallel is false, there should be no partial paths. */
-       Assert(sub_final_rel->consider_parallel ||
-                  sub_final_rel->partial_pathlist == NIL);
-
-       /* Same for partial paths. */
-       foreach(lc, sub_final_rel->partial_pathlist)
+       /* If outer rel allows parallelism, do same for partial paths. */
+       if (rel->consider_parallel && bms_is_empty(required_outer))
        {
-               Path       *subpath = (Path *) lfirst(lc);
-               List       *pathkeys;
-
-               /* Convert subpath's pathkeys to outer representation */
-               pathkeys = convert_subquery_pathkeys(root,
-                                                                                        rel,
-                                                                                        subpath->pathkeys,
-                                                                                        make_tlist_from_pathtarget(subpath->pathtarget));
+               /* If consider_parallel is false, there should be no partial paths. */
+               Assert(sub_final_rel->consider_parallel ||
+                          sub_final_rel->partial_pathlist == NIL);
 
-               /* Generate outer path using this subpath */
-               add_partial_path(rel, (Path *)
-                                                create_subqueryscan_path(root, rel, subpath,
-                                                                                                 pathkeys, required_outer));
+               /* Same for partial paths. */
+               foreach(lc, sub_final_rel->partial_pathlist)
+               {
+                       Path       *subpath = (Path *) lfirst(lc);
+                       List       *pathkeys;
+
+                       /* Convert subpath's pathkeys to outer representation */
+                       pathkeys = convert_subquery_pathkeys(root,
+                                                                                                rel,
+                                                                                                subpath->pathkeys,
+                                                                                                make_tlist_from_pathtarget(subpath->pathtarget));
+
+                       /* Generate outer path using this subpath */
+                       add_partial_path(rel, (Path *)
+                                                        create_subqueryscan_path(root, rel, subpath,
+                                                                                                         pathkeys,
+                                                                                                         required_outer));
+               }
        }
 }
 

diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 8d86e98adc118c2d7c6715ded8ba5d64a7c90017..61d0770f1089ddbcf030c3a3b76383e0f552ac3d 100644 (file)
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -330,7 +330,8 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
                 * to build a partial path for this relation.  But there's no point in
                 * considering any path but the cheapest.
                 */
-               if (final_rel->partial_pathlist != NIL)
+               if (rel->consider_parallel && bms_is_empty(rel->lateral_relids) &&
+                       final_rel->partial_pathlist != NIL)
                {
                        Path       *partial_subpath;
                        Path       *partial_path;

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 776c592dbe7d90733d0eb2664bc49ea7363ea2fe..e190ad49d16e64f690cb503123351afeb83ce34e 100644 (file)
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -770,6 +770,12 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path)
        /* Check for query cancel. */
        CHECK_FOR_INTERRUPTS();
 
+       /* Path to be added must be parallel safe. */
+       Assert(new_path->parallel_safe);
+
+       /* Relation should be OK for parallelism, too. */
+       Assert(parent_rel->consider_parallel);
+
        /*
         * As in add_path, throw out any paths which are dominated by the new
         * path, but throw out the new path if some existing path dominates it.

diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index fb209de5e9a2c81d651be302aa1c7c5883421592..a07cd50fcbfc84d5a58a4070972ab6fff3951019 100644 (file)
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -955,4 +955,23 @@ ORDER BY 1, 2, 3;
 ------------------------------+---------------------------+-------------+--------------
 (0 rows)
 
+-- test interation between subquery and partial_paths
+SET LOCAL min_parallel_table_scan_size TO 0;
+CREATE VIEW tenk1_vw_sec WITH (security_barrier) AS SELECT * FROM tenk1;
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM tenk1_vw_sec WHERE EXISTS (SELECT 1 WHERE unique1 = 0);
+                            QUERY PLAN                             
+-------------------------------------------------------------------
+ Subquery Scan on tenk1_vw_sec
+   Filter: (alternatives: SubPlan 1 or hashed SubPlan 2)
+   ->  Gather
+         Workers Planned: 4
+         ->  Parallel Index Only Scan using tenk1_unique1 on tenk1
+   SubPlan 1
+     ->  Result
+           One-Time Filter: (tenk1_vw_sec.unique1 = 0)
+   SubPlan 2
+     ->  Result
+(10 rows)
+
 rollback;

diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index ac26d68053c57ba00cf8d21cbc8e9435ad3b0532..7db75b0d2207cdd36a8ff282c070db3106881a39 100644 (file)
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -383,4 +383,10 @@ ORDER BY 1;
 SELECT * FROM information_schema.foreign_data_wrapper_options
 ORDER BY 1, 2, 3;
 
+-- test interation between subquery and partial_paths
+SET LOCAL min_parallel_table_scan_size TO 0;
+CREATE VIEW tenk1_vw_sec WITH (security_barrier) AS SELECT * FROM tenk1;
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM tenk1_vw_sec WHERE EXISTS (SELECT 1 WHERE unique1 = 0);
+
 rollback;