Subject: Fix a typo in pg_rotate_logfile

From: Bharath Rupireddy <[email-masked]>
Date: Tue, 13 Feb 2024 02:02:21 +0530

Hi,

I happened to notice a typo in pg_rotate_logfile in ipc/signalfuncs.c
- the hint message wrongly mentions that pg_logfile_rotate is part of
the core; which is actually not. pg_logfile_rotate is an adminpack's
1.0 SQL function dropped in 2.0. The core defines pg_rotate_logfile
SQL function instead, so use that. Here's a patch to fix the typo.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

---

From: Daniel Gustafsson <[email-masked]>
Date: Mon, 12 Feb 2024 21:39:06 +0100

> On 12 Feb 2024, at 21:32, Bharath Rupireddy <[email-masked]> wrote:

> SQL function instead, so use that. Here's a patch to fix the typo.

Nice catch!  This needs to be backpatched all the way down to 12 as that
function wen't away a long time ago (it was marked as deprecated all the way
back in 9.1).

--
Daniel Gustafsson

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 12 Feb 2024 14:46:13 -0600

> function wen't away a long time ago (it was marked as deprecated all the way

This is a bit strange because, with this patch, the HINT suggests using a
function with the same name as the one it lives in.  IIUC this is because
adminpack's pg_logfile_rotate() uses pg_rotate_logfile(), while core's
pg_rotate_logfile() uses pg_rotate_logfile_v2().  I suppose trying to
rename these might be more trouble than it's worth at this point, though...

-- 
Nathan Bossart

---

From: Daniel Gustafsson <[email-masked]>
Date: Mon, 12 Feb 2024 21:59:15 +0100

> On 12 Feb 2024, at 21:46, Nathan Bossart <[email-masked]> wrote:
> 

Yeah, I doubt that's worth the churn.

On that note though, we might want to consider just dropping it altogether in
v17 (while fixing the incorrect hint in backbranches)?  I can't imagine
adminpack 1.0 being in heavy use today, and skimming pgAdmin code it seems it's
only used in pgAdmin3 and not 4. Maybe it's time to simply drop old code?

--
Daniel Gustafsson

---

From: Bharath Rupireddy <[email-masked]>
Date: Tue, 13 Feb 2024 03:01:39 +0530

> only used in pgAdmin3 and not 4. Maybe it's time to simply drop old code?

https://codesearch.debian.net/search?q=pg_logfile_rotate&literal=1
shows no users for it though. There's pgadmin3 using it
https://github.com/search?q=repo%3Apgadmin-org%2Fpgadmin3%20pg_logfile_rotate&type=code,
however the repo is archived. Surprisingly, core has to maintain the
old code needed for adminpack 1.0 - pg_rotate_logfile_old SQL function
and pg_rotate_logfile function in signalfuncs.c. These things could
have been moved to adminpack.c back then and pointed CREATE FUNCTION
pg_catalog.pg_logfile_rotate() to use it from adminpack.c. If we
decide to remove adminpack 1.0 version completely, the 1.0 functions
pg_file_read, pg_file_length and pg_logfile_rotate will also go away
making adminpack code simpler.

Having said that, it's good to hear from others, preferably from
pgadmin developers - added Dave Page ([email-masked]) in here for
inputs.