On 2019/04/19 17:00, Amit Langote wrote:
> On 2019/04/19 2:25, Tom Lane wrote:
>> Amit Langote <langote_amit...@lab.ntt.co.jp> writes:
>>> Another idea is to teach explain.c about this special case of run-time
>>> pruning having pruned all child subplans even though appendplans contains
>>> one element to cater for targetlist accesses.  That is, Append will be
>>> displayed with "Subplans Removed: All" and no child subplans listed below
>>> it, even though appendplans[] has one.  David already said he didn't do in
>>> the first place to avoid PartitionPruneInfo details creeping into other
>>> modules, but maybe there's no other way?
>>
>> I tried simply removing the hack in nodeAppend.c (per quick-hack patch
>> below), and it gets through the core regression tests without a crash,
>> and with output diffs that seem fine to me.  However, that just shows that
>> we lack enough test coverage; we evidently have no regression cases where
>> an upper node needs to print Vars that are coming from a fully-pruned
>> Append.  Given the test case mentioned in this thread, I get
>>
>> regression=# explain verbose select * from t1 where dt = current_date + 400;
>>                  QUERY PLAN                  
>> ---------------------------------------------
>>  Append  (cost=0.00..198.42 rows=44 width=8)
>>    Subplans Removed: 4
>> (2 rows)
>>
>> which seems fine, but
>>
>> regression=# explain verbose select * from t1 where dt = current_date + 400 
>> order by id;
>> psql: server closed the connection unexpectedly
>>
>> It's dying trying to resolve Vars in the Sort node, of course.
> 
> Another approach, as I mentioned above, is to extend the hack that begins
> in nodeAppend.c (and nodeMergeAppend.c) into explain.c, as in the
> attached.  Then:
> 
> explain verbose select * from t1 where dt = current_date + 400 order by id;
>                     QUERY PLAN
> ───────────────────────────────────────────────────
>  Sort  (cost=199.62..199.73 rows=44 width=8)
>    Output: t1_1.id, t1_1.dt
>    Sort Key: t1_1.id
>    ->  Append  (cost=0.00..198.42 rows=44 width=8)
>          Subplans Removed: 4
> (5 rows)
> 
> It's pretty confusing to see t1_1 which has been pruned away, but you
> didn't seem very interested in the idea of teaching explain.c to use the
> original target list of plans like Append, MergeAppend, etc. that have
> child subplans.
> 
> Just a note: runtime pruning for MergeAppend is new in PG 12.

The patch I attached with the previous email didn't update the expected
output file.  Correct one attached.

Thanks,
Amit
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index a6c6de78f1..5a494fd4b6 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1981,16 +1981,27 @@ ExplainNode(PlanState *planstate, List *ancestors,
                                                           ancestors, es);
                        break;
                case T_Append:
-                       ExplainMemberNodes(((AppendState *) 
planstate)->appendplans,
-                                                          ((AppendState *) 
planstate)->as_nplans,
-                                                          list_length(((Append 
*) plan)->appendplans),
-                                                          ancestors, es);
+                       {
+                               AppendState *astate = (AppendState *) planstate;
+                               /* Runtime pruning may have pruned all 
partitions. */
+                               int     nsubnodes = astate->as_whichplan == 
NO_MATCHING_SUBPLANS ?
+                                                                       0 : 
astate->as_nplans;
+
+                               ExplainMemberNodes(astate->appendplans, 
nsubnodes,
+                                                                  
list_length(((Append *) plan)->appendplans),
+                                                                  ancestors, 
es);
+                       }
                        break;
                case T_MergeAppend:
-                       ExplainMemberNodes(((MergeAppendState *) 
planstate)->mergeplans,
-                                                          ((MergeAppendState 
*) planstate)->ms_nplans,
-                                                          
list_length(((MergeAppend *) plan)->mergeplans),
-                                                          ancestors, es);
+                       {
+                               MergeAppendState *mastate = (MergeAppendState 
*) planstate;
+                               /* Runtime pruning may have pruned all 
partitions. */
+                               int     nsubnodes = mastate->ms_noopscan ? 0 : 
mastate->ms_nplans;
+
+                               ExplainMemberNodes(mastate->mergeplans, 
nsubnodes,
+                                                                  
list_length(((MergeAppend *) plan)->mergeplans),
+                                                                  ancestors, 
es);
+                       }
                        break;
                case T_BitmapAnd:
                        ExplainMemberNodes(((BitmapAndState *) 
planstate)->bitmapplans,
diff --git a/src/backend/executor/nodeAppend.c 
b/src/backend/executor/nodeAppend.c
index f3be2429db..c12f5161e7 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -78,7 +78,6 @@ struct ParallelAppendState
 };
 
 #define INVALID_SUBPLAN_INDEX          -1
-#define NO_MATCHING_SUBPLANS           -2
 
 static TupleTableSlot *ExecAppend(PlanState *pstate);
 static bool choose_next_subplan_locally(AppendState *node);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index a5e4b7ef2e..40e019b395 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1155,6 +1155,9 @@ typedef struct ModifyTableState
  * ----------------
  */
 
+/* Exported for explain.c to use. */
+#define NO_MATCHING_SUBPLANS           -2
+
 struct AppendState;
 typedef struct AppendState AppendState;
 struct ParallelAppendState;
diff --git a/src/test/regress/expected/partition_prune.out 
b/src/test/regress/expected/partition_prune.out
index 0789b316eb..144e62b548 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2058,20 +2058,17 @@ select explain_parallel_append('execute ab_q5 (2, 3, 
3)');
 (19 rows)
 
 -- Try some params whose values do not belong to any partition.
--- We'll still get a single subplan in this case, but it should not be scanned.
 select explain_parallel_append('execute ab_q5 (33, 44, 55)');
-                            explain_parallel_append                            
--------------------------------------------------------------------------------
+                  explain_parallel_append                  
+-----------------------------------------------------------
  Finalize Aggregate (actual rows=1 loops=1)
    ->  Gather (actual rows=3 loops=1)
          Workers Planned: 2
          Workers Launched: 2
          ->  Partial Aggregate (actual rows=1 loops=3)
                ->  Parallel Append (actual rows=0 loops=N)
-                     Subplans Removed: 8
-                     ->  Parallel Seq Scan on ab_a1_b1 (never executed)
-                           Filter: ((b < 4) AND (a = ANY (ARRAY[$1, $2, $3])))
-(9 rows)
+                     Subplans Removed: 9
+(7 rows)
 
 -- Test Parallel Append with PARAM_EXEC Params
 select explain_parallel_append('select count(*) from ab where (a = (select 1) 
or a = (select 3)) and b = 2');
@@ -2961,16 +2958,13 @@ explain (analyze, costs off, summary off, timing off)  
execute q1 (2,2);
          Filter: (b = ANY (ARRAY[$1, $2]))
 (4 rows)
 
--- Try with no matching partitions. One subplan should remain in this case,
--- but it shouldn't be executed.
+-- Try with no matching partitions.
 explain (analyze, costs off, summary off, timing off)  execute q1 (0,0);
-                  QUERY PLAN                  
-----------------------------------------------
+           QUERY PLAN           
+--------------------------------
  Append (actual rows=0 loops=1)
-   Subplans Removed: 1
-   ->  Seq Scan on listp_1_1 (never executed)
-         Filter: (b = ANY (ARRAY[$1, $2]))
-(4 rows)
+   Subplans Removed: 2
+(2 rows)
 
 deallocate q1;
 -- Test more complex cases where a not-equal condition further eliminates 
partitions.
@@ -3011,15 +3005,12 @@ explain (analyze, costs off, summary off, timing off)  
execute q1 (1,2,2,0);
 (4 rows)
 
 -- Both partitions allowed by IN clause, then both excluded again by <> 
clauses.
--- One subplan will remain in this case, but it should not be executed.
 explain (analyze, costs off, summary off, timing off)  execute q1 (1,2,2,1);
-                               QUERY PLAN                                
--------------------------------------------------------------------------
+           QUERY PLAN           
+--------------------------------
  Append (actual rows=0 loops=1)
-   Subplans Removed: 1
-   ->  Seq Scan on listp_1_1 (never executed)
-         Filter: ((b = ANY (ARRAY[$1, $2])) AND ($3 <> b) AND ($4 <> b))
-(4 rows)
+   Subplans Removed: 2
+(2 rows)
 
 -- Ensure Params that evaluate to NULL properly prune away all partitions
 explain (analyze, costs off, summary off, timing off)
@@ -3168,14 +3159,12 @@ execute mt_q1(25);
 
 -- Ensure MergeAppend behaves correctly when no subplans match
 explain (analyze, costs off, summary off, timing off) execute mt_q1(35);
-                               QUERY PLAN                               
-------------------------------------------------------------------------
+              QUERY PLAN              
+--------------------------------------
  Merge Append (actual rows=0 loops=1)
    Sort Key: ma_test_p1.b
-   Subplans Removed: 2
-   ->  Index Scan using ma_test_p1_b_idx on ma_test_p1 (never executed)
-         Filter: ((a >= $1) AND ((a % 10) = 5))
-(5 rows)
+   Subplans Removed: 3
+(3 rows)
 
 execute mt_q1(35);
  a 
diff --git a/src/test/regress/sql/partition_prune.sql 
b/src/test/regress/sql/partition_prune.sql
index c30e58eef7..f2912d9afe 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -485,7 +485,6 @@ select explain_parallel_append('execute ab_q5 (1, 1, 1)');
 select explain_parallel_append('execute ab_q5 (2, 3, 3)');
 
 -- Try some params whose values do not belong to any partition.
--- We'll still get a single subplan in this case, but it should not be scanned.
 select explain_parallel_append('execute ab_q5 (33, 44, 55)');
 
 -- Test Parallel Append with PARAM_EXEC Params
@@ -726,8 +725,7 @@ explain (analyze, costs off, summary off, timing off)  
execute q1 (1,1);
 
 explain (analyze, costs off, summary off, timing off)  execute q1 (2,2);
 
--- Try with no matching partitions. One subplan should remain in this case,
--- but it shouldn't be executed.
+-- Try with no matching partitions.
 explain (analyze, costs off, summary off, timing off)  execute q1 (0,0);
 
 deallocate q1;
@@ -745,7 +743,6 @@ execute q1 (1,2,3,4);
 explain (analyze, costs off, summary off, timing off)  execute q1 (1,2,2,0);
 
 -- Both partitions allowed by IN clause, then both excluded again by <> 
clauses.
--- One subplan will remain in this case, but it should not be executed.
 explain (analyze, costs off, summary off, timing off)  execute q1 (1,2,2,1);
 
 -- Ensure Params that evaluate to NULL properly prune away all partitions

Reply via email to