Horton

Thread

Commits

  1. Fix some comments related to library unloading

  1. Fix a wrong comment in load_file()

    cca5507 <cca5507@qq.com> — 2024-12-23T02:58:49Z

    Hi,


The comment in load_file():


/* If the same shlib has previously been loaded, unload and reload it. */


But we currently don't support to unload a shlib.


Here is a small patch to fix it.


--
Regards,
ChangAo Chen

  2. Re: Fix a wrong comment in load_file()

    Michael Paquier <michael@paquier.xyz> — 2024-12-23T03:42:25Z

    On Mon, Dec 23, 2024 at 10:58:49AM +0800, cca5507 wrote:
> /* If the same shlib has previously been loaded, unload and reload it. */
> 
> But we currently don't support to unload a shlib.

  * This function loads a shlib file without looking up any particular
  * function in it.  If the same shlib has previously been loaded,
- * unload and reload it.
+ * we don't load it again.

Right.  This comment is misleading.

-	/* Load the shared library */
+	/* Load the shared library, unless we already did */
 	(void) internal_load_library(fullname);

This comment is still true, though.  internal_load_library() has some
logic to make sure that we don't load twice the same library on
repeated calls of the function as the file_list is checked.
--
Michael

  3. Re: Fix a wrong comment in load_file()

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-12-23T03:50:47Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Dec 23, 2024 at 10:58:49AM +0800, cca5507 wrote:
>   * This function loads a shlib file without looking up any particular
>   * function in it.  If the same shlib has previously been loaded,
> - * unload and reload it.
> + * we don't load it again.

> Right.  This comment is misleading.

I think it was once correct, decades ago.  Obviously we missed
fixing it when we ripped out the unload functionality.

> -	/* Load the shared library */
> +	/* Load the shared library, unless we already did */

> This comment is still true, though.

Seems like an improvement to me?

			regards, tom lane

  4. Re: Fix a wrong comment in load_file()

    Michael Paquier <michael@paquier.xyz> — 2024-12-23T04:10:34Z

    On Sun, Dec 22, 2024 at 10:50:47PM -0500, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> Right.  This comment is misleading.
> 
> I think it was once correct, decades ago.  Obviously we missed
> fixing it when we ripped out the unload functionality.

This was recent enough, and easy to miss.

>> -	/* Load the shared library */
>> +	/* Load the shared library, unless we already did */
> 
>> This comment is still true, though.
> 
> Seems like an improvement to me?

Fine by me.

While on it, I think that we should adjust all these ones too:
contrib/sepgsql/hooks.c: * Module load/unload callback
contrib/auto_explain/auto_explain.c:/* Saved hook values in case of
unload */
contrib/passwordcheck/passwordcheck.c:/* Saved hook value in case of
unload */
contrib/pg_stat_statements/pg_stat_statements.c:/* Saved hook values
in case of unload */

The saved hooks are not here to readjust the stack based on a reload,
just to make sure that the existing paths loaded are all taken.  I
would just remove the "in case of unload" part for the last three, and
"unload" for the first one.  Thoughts?
--
Michael

  5. Re: Fix a wrong comment in load_file()

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-12-23T04:15:31Z

    Michael Paquier <michael@paquier.xyz> writes:
> While on it, I think that we should adjust all these ones too:
> contrib/sepgsql/hooks.c: * Module load/unload callback
> contrib/auto_explain/auto_explain.c:/* Saved hook values in case of
> unload */
> contrib/passwordcheck/passwordcheck.c:/* Saved hook value in case of
> unload */
> contrib/pg_stat_statements/pg_stat_statements.c:/* Saved hook values
> in case of unload */

Check.

> The saved hooks are not here to readjust the stack based on a reload,
> just to make sure that the existing paths loaded are all taken.  I
> would just remove the "in case of unload" part for the last three, and
> "unload" for the first one.  Thoughts?

WFM

			regards, tom lane

  6. Re: Fix a wrong comment in load_file()

    cca5507 <cca5507@qq.com> — 2024-12-23T05:20:21Z

    &gt; The saved hooks are not here to readjust the stack based on a reload,
&gt; just to make sure that the existing paths loaded are all taken.&nbsp; I
&gt; would just remove the "in case of unload" part for the last three, and
&gt; "unload" for the first one.&nbsp; Thoughts?



LGTM.


--
Regards,
ChangAo Chen

  7. Re: Fix a wrong comment in load_file()

    Michael Paquier <michael@paquier.xyz> — 2024-12-23T06:51:39Z

    On Sun, Dec 22, 2024 at 11:15:31PM -0500, Tom Lane wrote:
> WFM

Okay, done this way then, thanks.
--
Michael