comments, skip storage of empty advice
authorRobert Haas <rhaas@postgresql.org>
Wed, 2 Jul 2025 17:50:23 +0000 (13:50 -0400)
committerRobert Haas <rhaas@postgresql.org>
Wed, 2 Jul 2025 17:50:23 +0000 (13:50 -0400)
contrib/pg_plan_advice/pg_plan_advice.c
contrib/pg_plan_advice/pgpa_collector.c

index 08ebdc0b260085673768847062b9712611e7f985..d002e9d26f7f34f7d2aa55ea86dc1d4290c5cca7 100644 (file)
@@ -179,6 +179,9 @@ pg_plan_advice_ExecutorStart(QueryDesc *queryDesc, int eflags)
                return standard_ExecutorStart(queryDesc, eflags);
 }
 
+/*
+ * Generate advice from a query plan and send it to the relevant collectors.
+ */
 static void
 pgpa_generate_advice(PlannedStmt *pstmt, const char *query_string)
 {
@@ -187,12 +190,17 @@ pgpa_generate_advice(PlannedStmt *pstmt, const char *query_string)
        ListCell   *lc;
        const char **rt_identifiers;
 
+       /* Construct unique identifiers for non-JOIN RTEs. */
        rt_identifiers = pgpa_create_identifiers_for_planned_stmt(pstmt);
 
+       /* Initialization. */
        memset(&context, 0, sizeof(pgpa_plan_walker_context));
        context.pstmt = pstmt;
+
+       /* Walk the main plan tree. */
        pgpa_plan_walker(&context, pstmt->planTree, 0, NULL, NULL);
 
+       /* Main plan tree walk won't reach subplans, so walk those. */
        foreach(lc, pstmt->subplans)
        {
                Plan       *plan = lfirst(lc);
@@ -201,7 +209,22 @@ pgpa_generate_advice(PlannedStmt *pstmt, const char *query_string)
                        pgpa_plan_walker(&context, plan, 0, NULL, NULL);
        }
 
+       /* Put advice into string form. */
        initStringInfo(&buf);
        pgpa_output_advice(&buf, &context, rt_identifiers);
-       pgpa_collect_advice(pstmt->queryId, query_string, buf.data);
+
+       /*
+        * If the advice string is non-empty, pass it to the collectors.
+        *
+        * A query such as SELECT 2+2 or SELECT * FROM generate_series(1,10)
+        * will not produce any advice, since there are no query planning decisions
+        * that can be influenced. It wouldn't exactly be wrong to record the
+        * query together with the empty advice string, but there doesn't seem to
+        * be much value in it, so skip it to save space.
+        *
+        * If this proves confusing to users, we might need to revist the behavior
+        * here.
+        */
+       if (buf.data[0] != '\0')
+               pgpa_collect_advice(pstmt->queryId, query_string, buf.data);
 }
index 81735182f4fa094d0a9750e0380847c6000d164f..59d37a67a6e9b66c2d96ae5600faac57b02aaa71 100644 (file)
@@ -1,3 +1,15 @@
+/*-------------------------------------------------------------------------
+ *
+ * pgpa_collector.c
+ *       collect advice into backend-local or shared memory
+ *
+ * Copyright (c) 2016-2025, PostgreSQL Global Development Group
+ *
+ *       contrib/pg_plan_advice/pgpa_collector.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
 #include "postgres.h"
 
 #include "pg_plan_advice.h"
@@ -15,6 +27,8 @@ PG_FUNCTION_INFO_V1(pg_get_collected_local_advice);
 #define ADVICE_CHUNK_SIZE              1024
 #define ADVICE_CHUNK_ARRAY_SIZE        64
 
+#define        PG_GET_ADVICE_COLUMNS   7
+
 typedef struct pgpa_collected_advice
 {
        Oid                     userid;                 /* user OID */
@@ -52,20 +66,24 @@ static pgpa_collected_advice *pgpa_make_collected_advice(Oid userid,
 static void pgpa_store_local_advice(pgpa_collected_advice *ca);
 static void pgpa_trim_local_advice(void);
 
+/* Helper function to extract the query string from pgpa_collected_advice */
 static inline const char *
 query_string(pgpa_collected_advice *ca)
 {
        return ca->textual_data;
 }
 
+/* Helper function to extract the advice string from pgpa_collected_advice */
 static inline const char *
 advice_string(pgpa_collected_advice *ca)
 {
        return ca->textual_data + ca->advice_offset;
 }
 
-#define        PG_GET_ADVICE_COLUMNS   7
-
+/*
+ * Store collected query advice into the local or shared advice collector,
+ * as appropriate.
+ */
 void
 pgpa_collect_advice(uint64 queryId, const char *query_string,
                                        const char *advice_string)