Обсуждение: small fix for pg_overexplain docs

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

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

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

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
theplanner. 
> (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

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
theplanner. 
> > (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

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
ofthe 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

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
ofthe 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

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

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

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

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
ofthe 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