add query features earlier before considering elided nodes
authorRobert Haas <rhaas@postgresql.org>
Tue, 15 Jul 2025 16:40:31 +0000 (12:40 -0400)
committerRobert Haas <rhaas@postgresql.org>
Tue, 15 Jul 2025 16:40:31 +0000 (12:40 -0400)
contrib/pg_plan_advice/pgpa_walker.c

index 694961eac866b943777e59427170b942985e04fc..3d8bd752afa56aa7bcd3341455708c38ed68dd8e 100644 (file)
@@ -35,33 +35,61 @@ pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan,
        bool            is_query_feature = false;
 
        /*
-        * Find all elided nodes for this Plan node.
+        * If this is a Gather or Gather Merge node, directly add it to the list
+        * of currently-active query features.
         *
-        * Also, because query features are specific to a subquery level, we reset
-        * the active_query_features list if this node has an elided SubqueryScan.
+        * Otherwise, check the future_query_features list to see whether this
+        * was previously identified as a plan node that needs to be treated as
+        * a query feature.
+        */
+       if (IsA(plan, Gather))
+       {
+               active_query_features =
+                       lappend(active_query_features,
+                                       pgpa_add_feature(context, PGPAQF_GATHER, plan));
+               is_query_feature = true;
+       }
+       else if (IsA(plan, GatherMerge))
+       {
+               active_query_features =
+                       lappend(active_query_features,
+                                       pgpa_add_feature(context, PGPAQF_GATHER_MERGE, plan));
+               is_query_feature = true;
+       }
+       else
+       {
+               foreach_ptr(pgpa_query_feature, qf, context->future_query_features)
+               {
+                       if (qf->plan == plan)
+                       {
+                               is_query_feature = true;
+                               active_query_features = lappend(active_query_features, qf);
+                               context->future_query_features =
+                                       list_delete_ptr(context->future_query_features, plan);
+                               break;
+                       }
+               }
+       }
+
+       /*
+        * Find all elided nodes for this Plan node.
         */
        foreach_node(ElidedNode, n, context->pstmt->elidedNodes)
        {
                if (n->plan_node_id == plan->plan_node_id)
-               {
                        elided_nodes = lappend(elided_nodes, n);
-                       if (n->elided_type == T_SubqueryScan)
-                               active_query_features = NIL;
-               }
        }
 
        /* If we found any elided_nodes, handle them. */
        if (elided_nodes != NIL)
        {
                int                     num_elided_nodes = list_length(elided_nodes);
+               ElidedNode *last_elided_node;
 
                /* XXX */
-               {
-                       ElidedNode *last_elided_node;
-
-                       last_elided_node = list_nth(elided_nodes, num_elided_nodes - 1);
-                       pgpa_qf_add_rtis(active_query_features, last_elided_node->relids);
-               }
+               last_elided_node = list_nth(elided_nodes, num_elided_nodes - 1);
+               pgpa_qf_add_rtis(active_query_features, last_elided_node->relids);
+               active_query_features = NIL;
 
                /*
                 * If there are multiple relids for the elided node, a clumped join
@@ -141,43 +169,6 @@ pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan,
                pgpa_unroll_join(context, plan, join_unroller,
                                                 &outer_join_unroller, &inner_join_unroller);
 
-       /*
-        * If this is a Gather or Gather Merge node, directly add it to the list
-        * of currently-active query features.
-        *
-        * Otherwise, check the future_query_features list to see whether this
-        * was previously identified as a plan node that needs to be treated as
-        * a query feature.
-        */
-       if (IsA(plan, Gather))
-       {
-               active_query_features =
-                       lappend(active_query_features,
-                                       pgpa_add_feature(context, PGPAQF_GATHER, plan));
-               is_query_feature = true;
-       }
-       else if (IsA(plan, GatherMerge))
-       {
-               active_query_features =
-                       lappend(active_query_features,
-                                       pgpa_add_feature(context, PGPAQF_GATHER_MERGE, plan));
-               is_query_feature = true;
-       }
-       else
-       {
-               foreach_ptr(pgpa_query_feature, qf, context->future_query_features)
-               {
-                       if (qf->plan == plan)
-                       {
-                               is_query_feature = true;
-                               active_query_features = lappend(active_query_features, qf);
-                               context->future_query_features =
-                                       list_delete_ptr(context->future_query_features, plan);
-                               break;
-                       }
-               }
-       }
-
        /* Add RTIs from the plan node to all active query features. */
        pgpa_qf_add_plan_rtis(active_query_features, plan);