Fix wrong reference in pg_overexplain's doc

Started by Julien Tachoires26 days ago8 messages

julien@tachoires.me

26 days ago

1 attachment(s)

Hi,

pg_overexplain's documentation mentions that the definition of
RangeTblEntry is in nodes/plannodes.h, which seems to be wrong as it
is defined in nodes/parsenodes.h. Please find a small patch fixing this.

Regards,

--
Julien Tachoires

Attachments:

v1-0001-Fix-wrong-reference-in-pg_overexplain-s-doc.patchtext/x-diff; charset=us-asciiDownload

From 9015494e2c778f8cd406e559232e38e136222554 Mon Sep 17 00:00:00 2001
From: Julien Tachoires <julien@tachoires.me>
Date: Thu, 18 Dec 2025 10:14:19 +0100
Subject: [PATCH] Fix wrong reference in pg_overexplain's doc.

This commit fixes the location of the definition of RangeTblEntry
in pg_overexplain's doc.
---
 doc/src/sgml/pgoverexplain.sgml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/src/sgml/pgoverexplain.sgml b/doc/src/sgml/pgoverexplain.sgml
index 377ddc8139e..0c8db13e4f0 100644
--- a/doc/src/sgml/pgoverexplain.sgml
+++ b/doc/src/sgml/pgoverexplain.sgml
@@ -186,7 +186,7 @@ LOAD 'pg_overexplain';
 
   <para>
    For more information about range table entries, see the definition of
-   <literal>RangeTblEntry</literal> in <literal>nodes/plannodes.h</literal>.
+   <literal>RangeTblEntry</literal> in <literal>nodes/parsenodes.h</literal>.
   </para>
  </sect2>
 
-- 
2.39.5

Shixin Wang

wang-shi-xin@outlook.com

26 days ago

In reply to: Julien Tachoires (#1)

Re: Fix wrong reference in pg_overexplain's doc

LGTM

The RangeTblEntry struct is indeed defined in parsenodes.h, not plannodes.h.
This patch corrects the incorrect reference.
All tests pass with make check-world.

Regards,
Shixin Wang

Fujii Masao

masao.fujii@gmail.com

25 days ago

In reply to: Julien Tachoires (#1)

Re: Fix wrong reference in pg_overexplain's doc

On Thu, Dec 18, 2025 at 6:23 PM Julien Tachoires <julien@tachoires.me> wrote:

Hi,

pg_overexplain's documentation mentions that the definition of
RangeTblEntry is in nodes/plannodes.h, which seems to be wrong as it
is defined in nodes/parsenodes.h. Please find a small patch fixing this.

Thanks for the patch! It looks good to me.

Barring any objections, I'll commit it and backpatch to v18,
where pg_overexplain was introduced.

Regards,

--
Fujii Masao

Fujii Masao

masao.fujii@gmail.com

22 days ago

In reply to: Fujii Masao (#3)

1 attachment(s)

Re: Fix wrong reference in pg_overexplain's doc

On Fri, Dec 19, 2025 at 12:16 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Dec 18, 2025 at 6:23 PM Julien Tachoires <julien@tachoires.me> wrote:

Hi,

pg_overexplain's documentation mentions that the definition of
RangeTblEntry is in nodes/plannodes.h, which seems to be wrong as it
is defined in nodes/parsenodes.h. Please find a small patch fixing this.

Thanks for the patch! It looks good to me.

Barring any objections, I'll commit it and backpatch to v18,
where pg_overexplain was introduced.

I've pushed the patch. Thanks!

After that, I noticed that the pg_overexplain docs uses the <literal> tag for
file names, struct names, and commands. It would be more appropriate to use
<filename>, <structname>, and <command> instead. I've attached a patch that
updates the documentation accordingly. Thoughts?

Regards,

--
Fujii Masao

Attachments:

v1-0001-doc-Use-proper-tags-in-pg_overexplain-documentati.patchapplication/octet-stream; name=v1-0001-doc-Use-proper-tags-in-pg_overexplain-documentati.patchDownload

From 5a329061448ffc41b631ebc58123436948e9b0df Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Mon, 22 Dec 2025 18:12:36 +0900
Subject: [PATCH v1] doc: Use proper tags in pg_overexplain documentation.

The pg_overexplain documentation previously used the <literal> tag for
some file names, struct names, and commands. Update the markup to
use the more appropriate tags: <filename>, <structname>, and <command>.

Backpatch to v18, where pg_overexplain was introduced.
---
 doc/src/sgml/pgoverexplain.sgml | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/pgoverexplain.sgml b/doc/src/sgml/pgoverexplain.sgml
index 0c8db13e4f0..e399c1cbad5 100644
--- a/doc/src/sgml/pgoverexplain.sgml
+++ b/doc/src/sgml/pgoverexplain.sgml
@@ -39,8 +39,8 @@ LOAD 'pg_overexplain';
    The <literal>DEBUG</literal> option displays miscellaneous information from
    the plan tree that is not normally shown because it is not expected to be
    of general interest. For each individual plan node, it will display the
-   following fields.  See <literal>Plan</literal> in
-   <literal>nodes/plannodes.h</literal> for additional documentation of these
+   following fields.  See <structname>Plan</structname> in
+   <filename>nodes/plannodes.h</filename> for additional documentation of these
    fields.
   </para>
 
@@ -82,8 +82,8 @@ LOAD 'pg_overexplain';
 
   <para>
    Once per query, the <literal>DEBUG</literal> option will display the
-   following fields. See <literal>PlannedStmt</literal> in
-   <literal>nodes/plannodes.h</literal> for additional detail.
+   following fields. See <structname>PlannedStmt</structname> in
+   <filename>nodes/plannodes.h</filename> for additional detail.
   </para>
 
   <itemizedlist>
@@ -97,7 +97,7 @@ LOAD 'pg_overexplain';
    <listitem>
     <para>
      <literal>Flags</literal>. A comma-separated list of Boolean structure
-     member names from the <literal>PlannedStmt</literal> that are set to
+     member names from the <structname>PlannedStmt</structname> that are set to
      <literal>true</literal>. It covers the following structure members:
      <literal>hasReturning</literal>, <literal>hasModifyingCTE</literal>,
      <literal>canSetTag</literal>, <literal>transientPlan</literal>,
@@ -177,7 +177,7 @@ LOAD 'pg_overexplain';
    table entry (e.g.  <literal>relation</literal>,
    <literal>subquery</literal>, or <literal>join</literal>), followed by the
    contents of various range table entry fields that are not normally part of
-   <literal>EXPLAIN</literal> output. Some of these fields are only displayed
+   <command>EXPLAIN</command> output. Some of these fields are only displayed
    for certain kinds of range table entries. For example,
    <literal>Eref</literal> is displayed for all types of range table entries,
    but <literal>CTE Name</literal> is displayed only for range table entries
@@ -186,7 +186,7 @@ LOAD 'pg_overexplain';
 
   <para>
    For more information about range table entries, see the definition of
-   <literal>RangeTblEntry</literal> in <literal>nodes/parsenodes.h</literal>.
+   <structname>RangeTblEntry</structname> in <filename>nodes/parsenodes.h</filename>.
   </para>
  </sect2>
 
-- 
2.51.2

Shixin Wang

wang-shi-xin@outlook.com

21 days ago

In reply to: Fujii Masao (#4)

Re: Fix wrong reference in pg_overexplain's doc

Hi Fujii-san

It would be more appropriate to use
<filename>, <structname>, and <command> instead.

```
-   following fields. See <literal>PlannedStmt</literal> in
-   <literal>nodes/plannodes.h</literal> for additional detail.
+   following fields. See <structname>PlannedStmt</structname> in
+   <filename>nodes/plannodes.h</filename> for additional detail.
```

Switching to <filename> here makes sense. While looking through the SGML files,
I noticed that the way header file paths are written is a bit inconsistent across the documentation.

In many places we use full paths including src/include, for example in fdw_handler.sgml and create_type.sgml:

```
in <filename>src/include/nodes/plannodes.h</filename>, and the comments for
<type>ExecRowMark</type> in <filename>src/include/nodes/execnodes.h</filename> for
```

But in a few files, such as pgoverexplain.sgml (this patch), spi.sgml,
and xfunc.sgml, the paths are written without the src/include/ prefix.

I’m fine with the change as-is; just wanted to ask whether you’d like to use
this patch to address that inconsistency, or keep the existing style in this file.

Regards,
Shixin Wang.

Fujii Masao

masao.fujii@gmail.com

21 days ago

In reply to: Shixin Wang (#5)

Re: Fix wrong reference in pg_overexplain's doc

On Tue, Dec 23, 2025 at 10:25 AM Shixin Wang <wang-shi-xin@outlook.com> wrote:

Hi Fujii-san

It would be more appropriate to use
<filename>, <structname>, and <command> instead.
```
-   following fields. See <literal>PlannedStmt</literal> in
-   <literal>nodes/plannodes.h</literal> for additional detail.
+   following fields. See <structname>PlannedStmt</structname> in
+   <filename>nodes/plannodes.h</filename> for additional detail.
```
Switching to <filename> here makes sense. While looking through the SGML files,

Thanks for the review!

I noticed that the way header file paths are written is a bit inconsistent across the documentation.

In many places we use full paths including src/include, for example in fdw_handler.sgml and create_type.sgml:

```
in <filename>src/include/nodes/plannodes.h</filename>, and the comments for
<type>ExecRowMark</type> in <filename>src/include/nodes/execnodes.h</filename> for
```

But in a few files, such as pgoverexplain.sgml (this patch), spi.sgml,
and xfunc.sgml, the paths are written without the src/include/ prefix.

Yes, and you can see similar inconsistencies elsewhere as well. For example,
paths to C source files are written inconsistently: they usually start with
src/backend or contrib, but some entries don't follow that convention.

I’m fine with the change as-is; just wanted to ask whether you’d like to use
this patch to address that inconsistency, or keep the existing style in this file.

At the moment, I don't plan to update the patch to address those
inconsistencies...

Regards,

--
Fujii Masao

Chao Li

li.evan.chao@gmail.com

20 days ago

In reply to: Fujii Masao (#6)

Re: Fix wrong reference in pg_overexplain's doc

On Dec 23, 2025, at 16:41, Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, Dec 23, 2025 at 10:25 AM Shixin Wang <wang-shi-xin@outlook.com> wrote:
Hi Fujii-san

It would be more appropriate to use
<filename>, <structname>, and <command> instead.
```
-   following fields. See <literal>PlannedStmt</literal> in
-   <literal>nodes/plannodes.h</literal> for additional detail.
+   following fields. See <structname>PlannedStmt</structname> in
+   <filename>nodes/plannodes.h</filename> for additional detail.
```
Switching to <filename> here makes sense. While looking through the SGML files,
Thanks for the review!

I noticed that the way header file paths are written is a bit inconsistent across the documentation.

In many places we use full paths including src/include, for example in fdw_handler.sgml and create_type.sgml:

```
in <filename>src/include/nodes/plannodes.h</filename>, and the comments for
<type>ExecRowMark</type> in <filename>src/include/nodes/execnodes.h</filename> for
```

But in a few files, such as pgoverexplain.sgml (this patch), spi.sgml,
and xfunc.sgml, the paths are written without the src/include/ prefix.

Yes, and you can see similar inconsistencies elsewhere as well. For example,
paths to C source files are written inconsistently: they usually start with
src/backend or contrib, but some entries don't follow that convention.

I’m fine with the change as-is; just wanted to ask whether you’d like to use
this patch to address that inconsistency, or keep the existing style in this file.

At the moment, I don't plan to update the patch to address those
inconsistencies...

The patch itself looks good to me. Following Shixin’s comment, I do see the inconsistencies of full path and relative path of .h files are referenced, if you don’t plan to address the inconvenience, I may file a patch for that.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Fujii Masao

masao.fujii@gmail.com

19 days ago

In reply to: Chao Li (#7)

Re: Fix wrong reference in pg_overexplain's doc

On Wed, Dec 24, 2025 at 8:54 AM Chao Li <li.evan.chao@gmail.com> wrote:

The patch itself looks good to me.

Thanks for the review! I've pushed the patch.

Following Shixin’s comment, I do see the inconsistencies of full path and relative path of .h files are referenced, if you don’t plan to address the inconvenience, I may file a patch for that.

Please feel free to work on that!

Regards,

--
Fujii Masao