More work on output stuff.

There is doubtless a lot of stuff still wrong here, but it's way
closer to what I actually want.

diff --git a/contrib/pg_plan_advice/pgpa_join.c b/contrib/pg_plan_advice/pgpa_join.c
index 0dc26adbe3cc4a440ceade84f5d94b177a7e8258..6e11882a35fc994e8c0a9067f2872fade55a7199 100644 (file)
--- a/contrib/pg_plan_advice/pgpa_join.c
+++ b/contrib/pg_plan_advice/pgpa_join.c
@@ -45,6 +45,10 @@ static void pgpa_fix_scan_or_clump_member(PlannedStmt *pstmt,
 static ElidedNode *pgpa_descend_node(PlannedStmt *pstmt, Plan **plan);
 static ElidedNode *pgpa_descend_any_gather(PlannedStmt *pstmt, Plan **plan);
 static ElidedNode *pgpa_descend_any_unique(PlannedStmt *pstmt, Plan **plan);
+
+static Bitmapset *pgpa_add_member_relids(Bitmapset *relids,
+                                                                                pgpa_join_member *member);
+
 static Index pgpa_scanrelid(Plan *plan);
 static Bitmapset *pgpa_relids(Plan *plan);
 
@@ -630,6 +634,40 @@ pgpa_descend_any_unique(PlannedStmt *pstmt, Plan **plan)
        return NULL;
 }
 
+/*
+ * Create a Bitmapset of all RTIs that we associated with an unrolled join.
+ *
+ * As usual, join RTIs are excluded.
+ */
+Bitmapset *
+pgpa_unrolled_join_all_relids(pgpa_unrolled_join *join)
+{
+       Bitmapset  *relids = NULL;
+
+       relids = pgpa_add_member_relids(relids, &join->outer);
+
+       for (int k = 0; k < join->ninner; ++k)
+               relids = pgpa_add_member_relids(relids, &join->inner[k]);
+
+       return relids;
+}
+
+/*
+ * Add all RTIs that we associate with a join member to the provided set,
+ * returning the result.
+ */
+static Bitmapset *
+pgpa_add_member_relids(Bitmapset *relids, pgpa_join_member *member)
+{
+       if (member->clump_join != NULL)
+               return bms_union(relids, member->clump_join->relids);
+       else if (member->unrolled_join != NULL)
+               return bms_union(relids,
+                                                pgpa_unrolled_join_all_relids(member->unrolled_join));
+       else
+               return bms_add_member(relids, member->rti);
+}
+
 /*
  * Update a pgpa_gathered_join to include RTIs scanned by the provided
  * plan node.

diff --git a/contrib/pg_plan_advice/pgpa_join.h b/contrib/pg_plan_advice/pgpa_join.h
index ee56e24a1c98543fe3f58fdeba6e2ae81a8ed8ff..0c9f3e949b814f6dc1f044e93e9e92ae27b5454c 100644 (file)
--- a/contrib/pg_plan_advice/pgpa_join.h
+++ b/contrib/pg_plan_advice/pgpa_join.h
@@ -43,7 +43,7 @@ typedef enum
        /* update NUM_PGPA_CLUMP_JOIN_STRATEGY if you add anything here */
 } pgpa_join_clump_strategy;
 
-#define NUM_PGPA_CLUMP_JOIN_STRATEGY   ((int) JSTRAT_CLUMP_PARTITIONWISE)
+#define NUM_PGPA_CLUMP_JOIN_STRATEGY   ((int) JSTRAT_CLUMP_PARTITIONWISE + 1)
 
 /*
  * All of the details we need regarding a clumped join.
@@ -74,7 +74,7 @@ typedef enum
        /* update NUM_PGPA_JOIN_STRATEGY if you add anything here */
 } pgpa_join_strategy;
 
-#define NUM_PGPA_JOIN_STRATEGY         ((int) JSTRAT_HASH_JOIN)
+#define NUM_PGPA_JOIN_STRATEGY         ((int) JSTRAT_HASH_JOIN + 1)
 
 /*
  * Within a pgpa_unrolled_join, we may need to refer to either to scans or to
@@ -158,6 +158,7 @@ extern void pgpa_unroll_join(PlannedStmt *pstmt, Plan *plan,
 extern pgpa_unrolled_join *pgpa_build_unrolled_join(PlannedStmt *pstmt,
                                                                                                        pgpa_join_unroller *join_unroller);
 extern void pgpa_destroy_join_unroller(pgpa_join_unroller *join_unroller);
+extern Bitmapset *pgpa_unrolled_join_all_relids(pgpa_unrolled_join *join);
 
 extern void pgpa_add_to_gathered_join(pgpa_gathered_join *gathered_join,
                                                                          Plan *plan);

diff --git a/contrib/pg_plan_advice/pgpa_output.c b/contrib/pg_plan_advice/pgpa_output.c
index de191c7d79678f92c8e6627cd851a1b0443620a6..0cc3936976ece3cf4cc03169ff2e21e161c78a93 100644 (file)
--- a/contrib/pg_plan_advice/pgpa_output.c
+++ b/contrib/pg_plan_advice/pgpa_output.c
@@ -18,12 +18,13 @@ typedef struct pgpa_output_context
 {
        const char **rt_identifiers;
        StringInfo      buf;
+       List       *unrolled_joins[NUM_PGPA_JOIN_STRATEGY];
+       List       *clumped_joins[NUM_PGPA_CLUMP_JOIN_STRATEGY];
        int                     wrap_column;
 } pgpa_output_context;
 
 static void pgpa_output_unrolled_join(pgpa_output_context *context,
-                                                                         pgpa_unrolled_join *join,
-                                                                         bool toplevel);
+                                                                         pgpa_unrolled_join *join);
 static void pgpa_output_join_member(pgpa_output_context *context,
                                                                        pgpa_join_member *member);
 static void pgpa_output_relations(pgpa_output_context *context, StringInfo buf,
@@ -41,6 +42,7 @@ pgpa_output_advice(StringInfo buf, pgpa_plan_walker_context *walker,
        ListCell   *lc;
        pgpa_output_context context;
 
+       memset(&context, 0, sizeof(pgpa_output_context));
        context.rt_identifiers = rt_identifiers;
        context.buf = buf;
        context.wrap_column = 79;       /* XXX */
@@ -51,19 +53,83 @@ pgpa_output_advice(StringInfo buf, pgpa_plan_walker_context *walker,
 
                if (buf->len > 0)
                        appendStringInfoChar(buf, '\n');
-               pgpa_output_unrolled_join(&context, ujoin, true);
+               appendStringInfo(context.buf, "JOIN_ORDER(");
+               pgpa_output_unrolled_join(&context, ujoin);
+               appendStringInfoChar(context.buf, ')');
        }
 
        foreach(lc, walker->clumped_joins)
        {
                pgpa_clumped_join *cjoin = lfirst(lc);
-               char       *cstrategy;
+
+               context.clumped_joins[cjoin->strategy] =
+                       lappend(context.clumped_joins[cjoin->strategy], cjoin->relids);
+       }
+
+       for (int s = 0; s < NUM_PGPA_JOIN_STRATEGY; ++s)
+       {
+               char *strategy = pgpa_cstring_join_strategy(s);
+               bool    first = true;
+
+               if (context.unrolled_joins[s] == NIL)
+                       continue;
+
+               if (buf->len > 0)
+                       appendStringInfoChar(buf, '\n');
+               appendStringInfo(buf, "%s(", strategy);
+
+               foreach(lc, context.unrolled_joins[s])
+               {
+                       Bitmapset *relids = lfirst(lc);
+
+                       if (first)
+                               first = false;
+                       else
+                       {
+                               pgpa_maybe_linebreak(context.buf, context.wrap_column);
+                               appendStringInfoChar(context.buf, ' ');
+                       }
+
+                       if (bms_membership(relids) == BMS_SINGLETON)
+                               pgpa_output_relations(&context, buf, relids);
+                       else
+                       {
+                               appendStringInfoChar(buf, '(');
+                               pgpa_output_relations(&context, buf, relids);
+                               appendStringInfoChar(buf, ')');
+                       }
+               }
+               appendStringInfoChar(buf, ')');
+       }
+
+       for (int c = 0; c < NUM_PGPA_CLUMP_JOIN_STRATEGY; ++c)
+       {
+               char *cstrategy = pgpa_cstring_join_clump_strategy(c);
+               bool    first = true;
+
+               if (context.clumped_joins[c] == NIL)
+                       continue;
 
                if (buf->len > 0)
                        appendStringInfoChar(buf, '\n');
-               cstrategy = pgpa_cstring_join_clump_strategy(cjoin->strategy);
                appendStringInfo(buf, "%s(", cstrategy);
-               pgpa_output_relations(&context, buf, cjoin->relids);
+
+               foreach(lc, context.clumped_joins[c])
+               {
+                       Bitmapset *relids = lfirst(lc);
+
+                       if (first)
+                               first = false;
+                       else
+                       {
+                               pgpa_maybe_linebreak(context.buf, context.wrap_column);
+                               appendStringInfoChar(context.buf, ' ');
+                       }
+
+                       appendStringInfoChar(buf, '(');
+                       pgpa_output_relations(&context, buf, relids);
+                       appendStringInfoChar(buf, ')');
+               }
                appendStringInfoChar(buf, ')');
        }
 
@@ -84,29 +150,28 @@ pgpa_output_advice(StringInfo buf, pgpa_plan_walker_context *walker,
 
 static void
 pgpa_output_unrolled_join(pgpa_output_context *context,
-                                                 pgpa_unrolled_join *join, bool toplevel)
+                                                 pgpa_unrolled_join *join)
 {
-       if (toplevel)
-               appendStringInfo(context->buf, "JOIN_ORDER(");
-       else
-               appendStringInfoChar(context->buf, '(');
-
        pgpa_output_join_member(context, &join->outer);
 
        for (int k = 0; k < join->ninner; ++k)
        {
-               char       *cstrategy;
-
-               pgpa_maybe_linebreak(context->buf, context->wrap_column);
-               cstrategy = pgpa_cstring_join_strategy(join->strategy[k]);
-               appendStringInfo(context->buf, " %s", cstrategy);
+               pgpa_join_member *member = &join->inner[k];
+               Bitmapset  *relids;
 
                pgpa_maybe_linebreak(context->buf, context->wrap_column);
                appendStringInfoChar(context->buf, ' ');
-               pgpa_output_join_member(context, &join->inner[k]);
-       }
+               pgpa_output_join_member(context, member);
 
-       appendStringInfoChar(context->buf, ')');
+               if (member->clump_join != NULL)
+                       relids = member->clump_join->relids;
+               else if (member->unrolled_join != NULL)
+                       relids = pgpa_unrolled_join_all_relids(member->unrolled_join);
+               else
+                       relids = bms_make_singleton(member->rti);
+               context->unrolled_joins[join->strategy[k]] =
+                       lappend(context->unrolled_joins[join->strategy[k]], relids);
+       }
 }
 
 static void
@@ -115,13 +180,20 @@ pgpa_output_join_member(pgpa_output_context *context,
 {
        if (member->clump_join != NULL)
        {
+               pgpa_clumped_join *cjoin = member->clump_join;
+
                appendStringInfoChar(context->buf, '{');
-               pgpa_output_relations(context, context->buf,
-                                                         member->clump_join->relids);
+               pgpa_output_relations(context, context->buf, cjoin->relids);
                appendStringInfoChar(context->buf, '}');
+               context->clumped_joins[cjoin->strategy] =
+                       lappend(context->clumped_joins[cjoin->strategy], cjoin->relids);
        }
        else if (member->unrolled_join != NULL)
-               pgpa_output_unrolled_join(context, member->unrolled_join, false);
+       {
+               appendStringInfoChar(context->buf, '(');
+               pgpa_output_unrolled_join(context, member->unrolled_join);
+               appendStringInfoChar(context->buf, ')');
+       }
        else
        {
                if (context->rt_identifiers[member->rti - 1] == NULL)