On Mon, Mar 2, 2026 at 3:09 PM David G. Johnston <[email protected]>
wrote:

> On Sun, Mar 1, 2026 at 9:10 PM David G. Johnston <
> [email protected]> wrote:
>
>> On Fri, Feb 27, 2026 at 6:16 PM David G. Johnston <
>> [email protected]> wrote:
>>
>>> On Fri, Feb 27, 2026 at 3:46 PM Robert Haas <[email protected]>
>>> wrote:
>>>
>>>> On Thu, Feb 26, 2026 at 8:55 AM Robert Haas <[email protected]>
>>>> wrote:
>>>> > Thanks, Alex, for the review.
>>>>
>>>> Here's v18. In addition to fixing the problems pointed out by Alex,
>>>> there are a couple of significant changes in this version.
>>>>
>>>>
>>> I have a mind to walk through the readmes and sgmls but its going to be
>>> in chunks.  Here's one for the readme for pg_plan_advice with a couple of
>>> preliminary sgml changes.
>>>
>>>
>> 0003 sgml focus with some readme.
>>
>>
> And now 0004 sgml (no readme):
>
>
Lastly, 0007 sgml (stash)

Placed entry in correct position on contrib page.

Expanded a bit on the security aspect comment.  I suppose there is some
indirect exposure via decisions being made implying table sizes or
records-per-FK...I left that unmentioned.

Added some explicit limitations to user-supplied values.

I would personally like to see "pg_set_stashed_advice" returning something
besides void.  I would usually go for text, but maybe in the interest of
i18n an integer would suffice.  +1 if a row was added, -1 if a row is
removed, or 0 for an update.  That would necessitate any other no-op being
an error.  Presently, supplying NULL for the query_id is not an error -
that seems like an oversight.  Same goes for supplying NULL for
stash_name.  If both of those cases produce errors (leaving NULL
advice_string being a remove indicator) the integer return seems like it
should work just fine.

Sorta feels like this module would appreciate advice strings having a
comment feature - so instead of just leaving an empty string behind saying
"we know, no advice needed" - it could contain actual content that isn't
applied advice.  Given the complexity of planning, comments seem warranted
in the advice themselves in any case.

Feels like this module needs export and import functions, especially given
the intro paragraph about the contents being volatile.  Or maybe a psql
script example producing a dynamic script file involving \gexec.

David J.
diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml
index 62af2b21fd7..8f09d728698 100644
--- a/doc/src/sgml/contrib.sgml
+++ b/doc/src/sgml/contrib.sgml
@@ -151,7 +151,6 @@ CREATE EXTENSION <replaceable>extension_name</replaceable>;
  &ltree;
  &pageinspect;
  &passwordcheck;
- &pgstashadvice;
  &pgbuffercache;
  &pgcollectadvice;
  &pgcrypto;
@@ -161,6 +160,7 @@ CREATE EXTENSION <replaceable>extension_name</replaceable>;
  &pgplanadvice;
  &pgprewarm;
  &pgrowlocks;
+ &pgstashadvice;
  &pgstatstatements;
  &pgstattuple;
  &pgsurgery;
diff --git a/doc/src/sgml/pgstashadvice.sgml b/doc/src/sgml/pgstashadvice.sgml
index d1878f4f7c1..79b89beba38 100644
--- a/doc/src/sgml/pgstashadvice.sgml
+++ b/doc/src/sgml/pgstashadvice.sgml
@@ -10,7 +10,7 @@
  <para>
   The <filename>pg_stash_advice</filename> extension allows you to stash
   <link linkend="pgplanadvice">plan advice</link> strings in dynamic
-  shared memory where they can be automatically applied.  An
+  shared memory where they can be automatically applied. An
   <literal>advice stash</literal> is a mapping from
   <link linkend="guc-compute-query-id">query identifiers</link> to plan advice
   strings. Whenever a session is asked to plan a query whose query ID appears
@@ -25,7 +25,7 @@
   In order to use this module, you will need to execute
   <literal>CREATE EXTENSION pg_stash_advice</literal> in at least
   one database, so that you have access to the SQL functions to manage
-  advice stashes.  You will also need the <literal>pg_stash_advice</literal>
+  advice stashes. You will also need the <literal>pg_stash_advice</literal>
   module to be loaded in all sessions where you want this module to
   automatically apply advice. It will usually be best to do this by adding
   <literal>pg_stash_advice</literal> to
@@ -54,12 +54,12 @@
   string that you wish to store for each query. One way to do this is to use
   <literal>EXPLAIN</literal>: the <literal>VERBOSE</literal> option will
   show the query ID, and the <literal>PLAN_ADVICE</literal> option will
-  show plan advice.  <xref linkend="pgcollectadvice" /> can be used to
+  show plan advice. <xref linkend="pgcollectadvice" /> can be used to
   obtain this information for an entire workload, although care must be
   taken since it can use up a lot of memory very quickly. Query identifiers can
   also be obtained through tools such as <xref linkend="pgstatstatements" />
   or <xref linkend="monitoring-pg-stat-activity-view" />, but these tools
-  will not provide plan advice strings.  Note that
+  will not provide plan advice strings. Note that
   <xref linkend="guc-compute-query-id" /> must be enabled for query
   identifiers to be computed; if set to <literal>auto</literal>, loading
   <literal>pg_stash_advice</literal> will enable it automatically.
@@ -84,7 +84,9 @@
   <literal>pg_stash_advice.stash_name</literal> for their session, and this
   may reveal the contents of any advice stash with that name. Users should
   assume that information embedded in stashed advice strings may become visible
-  to nonprivileged users.
+  to nonprivileged users. However, advice targets are the only explicit
+  userspace data present in advice strings and most catalog contents are
+  considered non-privileged in PostgreSQL.
  </para>
 
  <sect2 id="pgstashadvice-functions">
@@ -102,7 +104,8 @@
 
     <listitem>
      <para>
-      Creates a new, empty advice stash with the given name.
+      Creates a new, empty advice stash with the given name, which must not be
+      zero length.
      </para>
     </listitem>
    </varlistentry>
@@ -134,10 +137,13 @@
     <listitem>
      <para>
       Stores an advice string in the named advice stash, associated with
-      the given query identifier.  If an entry for that query identifier
-      already exists in the stash, it is replaced.  If
+      the given query identifier. If an entry for that query identifier
+      already exists in the stash, it is replaced. If
       <parameter>advice_string</parameter> is <literal>NULL</literal>,
-      any existing entry for that query identifier is removed.
+      any existing entry for that query identifier is removed. A zero length advice
+      string is stored in the stash. An error is raised if stash_name does
+      not exist. Passing NULL for the query identifier results in a silent no-op,
+      and passing 0 is not permitted, but otherwise, any bigint value is accepted.
      </para>
     </listitem>
    </varlistentry>
@@ -170,7 +176,7 @@
 
     <listitem>
      <para>
-      Returns one row for each entry in the named advice stash.  If
+      Returns one row for each entry in the named advice stash. If
       <parameter>stash_name</parameter> is <literal>NULL</literal>, returns
       entries from all stashes.
      </para>
@@ -197,7 +203,7 @@
     <listitem>
      <para>
       Specifies the name of the advice stash to consult during query
-      planning.  The default value is the empty string, which disables
+      planning. The default value is the empty string, which disables
       this module.
      </para>
     </listitem>

Reply via email to