small fix for pg_overexplain docs

Started by Nathan Bossart6 months ago12 messages
#1Nathan Bossart
nathandbossart@gmail.com
1 attachment(s)

I noticed that pg_overexplain's documentation leads off with this sentence:

The pg_overexplain extends EXPLAIN with...

Presumably that's meant to be something like:

The pg_overexplain MODULE extends EXPLAIN with...

Another pattern in adjacent pages is to leave out the "The":

pg_overexplain extends EXPLAIN with...

The attached patch removes the "The". If there are no objections, I will
commit this shortly.

--
nathan

Attachments:

fix_overexplain_docs.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/pgoverexplain.sgml b/doc/src/sgml/pgoverexplain.sgml
index 21930fbd3bd..0e89cd1abd2 100644
--- a/doc/src/sgml/pgoverexplain.sgml
+++ b/doc/src/sgml/pgoverexplain.sgml
@@ -8,7 +8,7 @@
  </indexterm>
 
  <para>
-  The <filename>pg_overexplain</filename> extends <command>EXPLAIN</command>
+  <filename>pg_overexplain</filename> extends <command>EXPLAIN</command>
   with new options that provide additional output. It is mostly intended to
   assist with debugging of and development of the planner, rather than for
   general use. Since this module displays internal details of planner data
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#1)
Re: small fix for pg_overexplain docs

Nathan Bossart <nathandbossart@gmail.com> writes:

The attached patch removes the "The". If there are no objections, I will
commit this shortly.

When is "shortly"? I plan to wrap beta2 in about an hour.

regards, tom lane

#3Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#2)
Re: small fix for pg_overexplain docs

On Mon, Jul 14, 2025 at 03:14:35PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

The attached patch removes the "The". If there are no objections, I will
commit this shortly.

When is "shortly"? I plan to wrap beta2 in about an hour.

Ah, shoot, I forgot v18 was frozen. I'll wait for the tags. Thanks for
reminding me.

--
nathan

#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Nathan Bossart (#1)
Re: small fix for pg_overexplain docs

On Mon, Jul 14, 2025 at 12:10 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:

I noticed that pg_overexplain's documentation leads off with this sentence:

The pg_overexplain extends EXPLAIN with...

Presumably that's meant to be something like:

The pg_overexplain MODULE extends EXPLAIN with...

Another pattern in adjacent pages is to leave out the "The":

pg_overexplain extends EXPLAIN with...

The attached patch removes the "The". If there are no objections, I will
commit this shortly.

Randomly picking 3 other modules turns up:

The pgrowlocks module provides...
The pgcrypto module provides cryptographic...
This module implements the hstore data type for storing sets...
The pg_logicalinspect module provides SQL...

A leading "The" and using the word "module" seems to be a more consistent
choice.

The pg_overexplain extends EXPLAIN with new options that provide...
Suggest instead:
The pg_overexplain module provides EXPLAIN with new options intended to
assist with debugging of and development of the planner.
(replacing the first two sentences of the existing paragraph)

David J.

#5Robert Treat
rob@xzilla.net
In reply to: David G. Johnston (#4)
Re: small fix for pg_overexplain docs

On Mon, Jul 14, 2025 at 3:22 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Mon, Jul 14, 2025 at 12:10 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

I noticed that pg_overexplain's documentation leads off with this sentence:

The pg_overexplain extends EXPLAIN with...

Presumably that's meant to be something like:

The pg_overexplain MODULE extends EXPLAIN with...

Another pattern in adjacent pages is to leave out the "The":

pg_overexplain extends EXPLAIN with...

The attached patch removes the "The". If there are no objections, I will
commit this shortly.

Randomly picking 3 other modules turns up:

The pgrowlocks module provides...
The pgcrypto module provides cryptographic...
This module implements the hstore data type for storing sets...
The pg_logicalinspect module provides SQL...

A leading "The" and using the word "module" seems to be a more consistent choice.

The pg_overexplain extends EXPLAIN with new options that provide...
Suggest instead:
The pg_overexplain module provides EXPLAIN with new options intended to assist with debugging of and development of the planner.
(replacing the first two sentences of the existing paragraph)

+1 for consistency, though I'd argue "intended to assist with
debugging and development of the planner" would be easier to read (in
either one sentence or two sentence format).

Robert Treat
https://xzilla.net

#6Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Robert Treat (#5)
Re: small fix for pg_overexplain docs

On Tue, 15 Jul 2025 at 04:17, Robert Treat <rob@xzilla.net> wrote:

On Mon, Jul 14, 2025 at 3:22 PM David G. Johnston

The pg_overexplain extends EXPLAIN with new options that provide...
Suggest instead:
The pg_overexplain module provides EXPLAIN with new options intended to assist with debugging of and development of the planner.
(replacing the first two sentences of the existing paragraph)

+1 for consistency, though I'd argue "intended to assist with
debugging and development of the planner" would be easier to read (in
either one sentence or two sentence format).

+1. I find that easier to read.

Something else that's missing is instructions on how to load the
module, which usually follows this first paragraph.

Regards,
Dean

#7Nathan Bossart
nathandbossart@gmail.com
In reply to: Dean Rasheed (#6)
Re: small fix for pg_overexplain docs

On Tue, Jul 15, 2025 at 04:14:44PM +0100, Dean Rasheed wrote:

On Tue, 15 Jul 2025 at 04:17, Robert Treat <rob@xzilla.net> wrote:

On Mon, Jul 14, 2025 at 3:22 PM David G. Johnston

The pg_overexplain extends EXPLAIN with new options that provide...
Suggest instead:
The pg_overexplain module provides EXPLAIN with new options intended to assist with debugging of and development of the planner.
(replacing the first two sentences of the existing paragraph)

+1 for consistency, though I'd argue "intended to assist with
debugging and development of the planner" would be easier to read (in
either one sentence or two sentence format).

+1. I find that easier to read.

I disagree. While it might sound nicer, the extra "of" establishes
parallelism to make it clear that the module is intended for debugging of
the planner and not debugging in general. I also see little need to
combine the two sentences. The first establishes at a high-level what the
module does, and the second provides more details. Combining the two as
proposed also loses context, i.e., "provide additional output" and "rather
than for general use" are removed. We could try to retain those pieces,
but then we're just combining two medium-sized sentences into a long one
that is more difficult to read.

FWIW there are counterexamples to the "The XXX module..." pattern, too
(e.g., auth_delay, basebackup_to_shell, basic_archive, bloom). But I'm
fine with using it here since that seems to be the preference.

Something else that's missing is instructions on how to load the
module, which usually follows this first paragraph.

Good point. That should probably look something like this, which is mostly
lifted from auto_explain:

To use it, simply load it into the server. You can load it into an
individual session:

LOAD 'pg_overexplain';

(You must be superuser to do that.) You can also preload it into some
or all sessions by including pg_overexplain in
session_preload_libraries or shared_preload_libraries in
postgresql.conf.

As an aside, the superuser note isn't totally true because administrators
can put stuff in $libdir/plugins/ to allow non-superusers to LOAD it.
Maybe we should just remove the superuser note in the module docs.

--
nathan

#8Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Nathan Bossart (#7)
Re: small fix for pg_overexplain docs

On Wed, 16 Jul 2025 at 18:26, Nathan Bossart <nathandbossart@gmail.com> wrote:

On Tue, Jul 15, 2025 at 04:14:44PM +0100, Dean Rasheed wrote:

On Tue, 15 Jul 2025 at 04:17, Robert Treat <rob@xzilla.net> wrote:

On Mon, Jul 14, 2025 at 3:22 PM David G. Johnston

The pg_overexplain extends EXPLAIN with new options that provide...
Suggest instead:
The pg_overexplain module provides EXPLAIN with new options intended to assist with debugging of and development of the planner.
(replacing the first two sentences of the existing paragraph)

+1 for consistency, though I'd argue "intended to assist with
debugging and development of the planner" would be easier to read (in
either one sentence or two sentence format).

+1. I find that easier to read.

I disagree. While it might sound nicer, the extra "of" establishes
parallelism to make it clear that the module is intended for debugging of
the planner and not debugging in general.

Hmm, I would normally only do that if it was 2 different prepositions,
and even then, I find it kind-of clunky. But I don't feel particularly
strongly one way or the other.

Something else that's missing is instructions on how to load the
module, which usually follows this first paragraph.

Good point. That should probably look something like this, which is mostly
lifted from auto_explain:

To use it, simply load it into the server. You can load it into an
individual session:

LOAD 'pg_overexplain';

(You must be superuser to do that.) You can also preload it into some
or all sessions by including pg_overexplain in
session_preload_libraries or shared_preload_libraries in
postgresql.conf.

Seems reasonable.

As an aside, the superuser note isn't totally true because administrators
can put stuff in $libdir/plugins/ to allow non-superusers to LOAD it.
Maybe we should just remove the superuser note in the module docs.

Maybe. It's kind-of annoying that all the modules that aren't
extensions use different text. Maybe there are genuine differences --
I didn't look too closely. It would be nice if we just had one
standard description that they all could refer to, but if that's
possible, it's a much bigger patch than you probably want to worry
about here, so I'm happy to go with your text above.

Regards,
Dean

#9Nathan Bossart
nathandbossart@gmail.com
In reply to: Dean Rasheed (#8)
1 attachment(s)
Re: small fix for pg_overexplain docs

On Wed, Jul 16, 2025 at 07:45:05PM +0100, Dean Rasheed wrote:

Maybe. It's kind-of annoying that all the modules that aren't
extensions use different text. Maybe there are genuine differences --
I didn't look too closely. It would be nice if we just had one
standard description that they all could refer to, but if that's
possible, it's a much bigger patch than you probably want to worry
about here, so I'm happy to go with your text above.

Okay, here is a new version of the patch.

--
nathan

Attachments:

v2-0001-Minor-pg_overexplain-doc-adjustments.patchtext/plain; charset=us-asciiDownload
From 56a73791f5712e1c5db45dc3bb6020654ff77e1b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 16 Jul 2025 16:21:50 -0500
Subject: [PATCH v2 1/1] Minor pg_overexplain doc adjustments.

---
 doc/src/sgml/pgoverexplain.sgml | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/pgoverexplain.sgml b/doc/src/sgml/pgoverexplain.sgml
index 21930fbd3bd..ab8f05e8a23 100644
--- a/doc/src/sgml/pgoverexplain.sgml
+++ b/doc/src/sgml/pgoverexplain.sgml
@@ -8,7 +8,7 @@
  </indexterm>
 
  <para>
-  The <filename>pg_overexplain</filename> extends <command>EXPLAIN</command>
+  The <filename>pg_overexplain</filename> module extends <command>EXPLAIN</command>
   with new options that provide additional output. It is mostly intended to
   assist with debugging of and development of the planner, rather than for
   general use. Since this module displays internal details of planner data
@@ -17,6 +17,21 @@
   often as) those data structures change.
  </para>
 
+ <para>
+  To use it, simply load it into the server.  You can load it into an
+  individual session:
+
+<programlisting>
+LOAD 'pg_overexplain';
+</programlisting>
+
+  (You must be superuser to do that.)  You can also preload it into some or all
+  sessions by including <literal>pg_overexplain</literal> in
+  <xref linkend="guc-session-preload-libraries"/> or
+  <xref linkend="guc-shared-preload-libraries"/> in
+  <filename>postgresql.conf</filename>.
+ </para>
+
  <sect2 id="pgoverexplain-debug">
   <title>EXPLAIN (DEBUG)</title>
 
-- 
2.39.5 (Apple Git-154)

#10Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Nathan Bossart (#9)
Re: small fix for pg_overexplain docs

On Wed, 16 Jul 2025 at 22:22, Nathan Bossart <nathandbossart@gmail.com> wrote:

Okay, here is a new version of the patch.

LGTM.

Regards,
Dean

#11Nathan Bossart
nathandbossart@gmail.com
In reply to: Dean Rasheed (#10)
Re: small fix for pg_overexplain docs

On Wed, Jul 16, 2025 at 10:47:03PM +0100, Dean Rasheed wrote:

LGTM.

Committed. I ended up leaving out the note about needing to be a superuser
to use LOAD, mostly because I found myself spending way too much time
justifying it in the commit message.

--
nathan

#12Robert Treat
rob@xzilla.net
In reply to: Nathan Bossart (#7)
Re: small fix for pg_overexplain docs

On Wed, Jul 16, 2025 at 1:26 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

On Tue, Jul 15, 2025 at 04:14:44PM +0100, Dean Rasheed wrote:

On Tue, 15 Jul 2025 at 04:17, Robert Treat <rob@xzilla.net> wrote:

On Mon, Jul 14, 2025 at 3:22 PM David G. Johnston

The pg_overexplain extends EXPLAIN with new options that provide...
Suggest instead:
The pg_overexplain module provides EXPLAIN with new options intended to assist with debugging of and development of the planner.
(replacing the first two sentences of the existing paragraph)

+1 for consistency, though I'd argue "intended to assist with
debugging and development of the planner" would be easier to read (in
either one sentence or two sentence format).

+1. I find that easier to read.

I disagree. While it might sound nicer, the extra "of" establishes
parallelism to make it clear that the module is intended for debugging of
the planner and not debugging in general.

Grammatically speaking, the "and" establishes parallelism in the
sentence, not the extra "of", which is superfluous (and imho is what
subconsciously causes the existing text to feel more awkward). Of
course, you could also flip it to say "... to assist with planner
debugging and development".

*shrug*

Robert Treat
https://xzilla.net