Horton

Thread

Commits

  1. Document "relrewrite" at the top of heap_create_with_catalog()

  2. Rearrange order of operations in heap_drop_with_catalog and index_drop

  3. Allow removal of system-named pg_* temp tables. Rename temp file/dir as

  1. relrewrite not documented at the top of heap_create_with_catalog()

    Michael Paquier <michael@paquier.xyz> — 2025-06-16T06:49:18Z

    Hi all,

While looking at the heap code, I've found that relrewrite, parameter
used a trick to keep a link to the original relation rewritten, is not
documented at the top of heap_create_with_catalog() contrary to all
its other parameters.

A simple patch is attached to document that.

Thanks,
--
Michael

  2. Re: relrewrite not documented at the top of heap_create_with_catalog()

    Steven Niu <niushiji@gmail.com> — 2025-06-16T08:51:46Z

    Hi, Michael,

The change looks good to me.

I made more change based on your patch to combine the old comment.

Regards,
Steven

Michael Paquier <michael@paquier.xyz> 于2025年6月16日周一 14:49写道：

> Hi all,
>
> While looking at the heap code, I've found that relrewrite, parameter
> used a trick to keep a link to the original relation rewritten, is not
> documented at the top of heap_create_with_catalog() contrary to all
> its other parameters.
>
> A simple patch is attached to document that.
>
> Thanks,
> --
> Michael
>

  3. Re: relrewrite not documented at the top of heap_create_with_catalog()

    Yugo Nagata <nagata@sraoss.co.jp> — 2025-06-16T13:37:43Z

    On Mon, 16 Jun 2025 16:51:46 +0800
Steven Niu <niushiji@gmail.com> wrote:

> Hi, Michael,
> 
> The change looks good to me.
> 
> I made more change based on your patch to combine the old comment.

I think it's a good idea to move the description of heap_create_with_catalog
directly above the function itself, as it seems better to keep the explanation
close to the function definition rather than placing it before related functions.
A similar change was made to heap_drop_with_catalog in commit 49ce6fff1d34.

I'm not sure whether this should be merged into the original patch, though.

I have a very minor comment on the initial patch.

+ *	relrewrite: link to original relation during a table rewrite.

I wonder if the period at the end is unnecessary, since this is not a full
sentence, and for consistency with the other lines.

Best regards,
Yugo Nagata

> Regards,
> Steven
> 
> Michael Paquier <michael@paquier.xyz> 于2025年6月16日周一 14:49写道：
> 
> > Hi all,
> >
> > While looking at the heap code, I've found that relrewrite, parameter
> > used a trick to keep a link to the original relation rewritten, is not
> > documented at the top of heap_create_with_catalog() contrary to all
> > its other parameters.
> >
> > A simple patch is attached to document that.
> >
> > Thanks,
> > --
> > Michael
> >


-- 
Yugo Nagata <nagata@sraoss.co.jp>

  4. Re: relrewrite not documented at the top of heap_create_with_catalog()

    Michael Paquier <michael@paquier.xyz> — 2025-06-18T02:12:48Z

    On Mon, Jun 16, 2025 at 10:37:43PM +0900, Yugo Nagata wrote:
> I think it's a good idea to move the description of heap_create_with_catalog
> directly above the function itself, as it seems better to keep the explanation
> close to the function definition rather than placing it before related functions.
> A similar change was made to heap_drop_with_catalog in commit 49ce6fff1d34.
> 
> I'm not sure whether this should be merged into the original patch, though.

More to the point: the whole comment block feels incomplete.  It is
missing for example a couple of steps, like between 2) and 3) due to
the introduction of binary upgrade logic.  The logic of the function
is much better guessed while reading through the code, IMO, so I would
suggest to just remove the whole comment block, with the part about
the "see comments above." at the top of the routine definition as we
are proving to not be good in maintaining it.  There is perhaps a bit
more that could be cleaned up; that's some legacy code back from the
original Postgres95.

> + *	relrewrite: link to original relation during a table rewrite.
> 
> I wonder if the period at the end is unnecessary, since this is not a full
> sentence, and for consistency with the other lines.

On consistency grounds, that makes sense.  Removed it and applied this
comment fix.
--
Michael

  5. Re: relrewrite not documented at the top of heap_create_with_catalog()

    Yugo Nagata <nagata@sraoss.co.jp> — 2025-06-18T03:04:01Z

    On Wed, 18 Jun 2025 11:12:48 +0900
Michael Paquier <michael@paquier.xyz> wrote:

> On Mon, Jun 16, 2025 at 10:37:43PM +0900, Yugo Nagata wrote:
> > I think it's a good idea to move the description of heap_create_with_catalog
> > directly above the function itself, as it seems better to keep the explanation
> > close to the function definition rather than placing it before related functions.
> > A similar change was made to heap_drop_with_catalog in commit 49ce6fff1d34.
> > 
> > I'm not sure whether this should be merged into the original patch, though.
> 
> More to the point: the whole comment block feels incomplete.  It is
> missing for example a couple of steps, like between 2) and 3) due to
> the introduction of binary upgrade logic.  The logic of the function
> is much better guessed while reading through the code, IMO, so I would
> suggest to just remove the whole comment block, with the part about
> the "see comments above." at the top of the routine definition as we
> are proving to not be good in maintaining it.  There is perhaps a bit
> more that could be cleaned up; that's some legacy code back from the
> original Postgres95.

I agree that the comment block should be removed, as it doesn't seem
so useful. I previously mentioned commit 49ce6fff1d34, which moved the comments
on heap_drop_with_catalog, but I also noticed that the step-by-step overview had
already been removed in commit 448eb0837f7a8, due to changes in the function’s logic.

Best regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>