Horton

Thread

Commits

  1. pg_resetwal: Fix incorrect error message related to pg_wal/summaries/

  1. pg_resetwal: Fix wrong directory in log output

    Tianchen Zhang <zhang_tian_chen@163.com> — 2026-02-03T01:58:19Z

    Hi hackers,

There is a misuse of macro when we output directory information in KillExistingWALSummaries(), pg_resetwal.c. Which should be WALSUMMARYDIR instead of ARCHSTATDIR.

Best regards,
Tianchen Zhang

  2. Re: pg_resetwal: Fix wrong directory in log output

    Michael Paquier <michael@paquier.xyz> — 2026-02-03T02:04:48Z

    On Tue, Feb 03, 2026 at 01:58:19AM +0000, zhang_tian_chen@163.com wrote:
> There is a misuse of macro when we output directory information in
> KillExistingWALSummaries(), pg_resetwal.c. Which should be
> WALSUMMARYDIR instead of ARCHSTATDIR.

Eh.  Good find.
--
Michael

  3. Re: pg_resetwal: Fix wrong directory in log output

    Chao Li <li.evan.chao@gmail.com> — 2026-02-03T02:13:53Z

    
> On Feb 3, 2026, at 09:58, zhang_tian_chen@163.com wrote:
> 
> Hi hackers,
> 
> There is a misuse of macro when we output directory information in KillExistingWALSummaries(), pg_resetwal.c. Which should be WALSUMMARYDIR instead of ARCHSTATDIR.
> 
> Best regards,
> Tianchen Zhang<v1-0001-Fix-incorrect-directory-macro-in-KillExistingWALS.patch>

Indeed a bug. Looking at the code:

```
/*
 * Remove existing WAL summary files
 */
static void
KillExistingWALSummaries(void)
{
	DIR		   *xldir;

	xldir = opendir(WALSUMMARYDIR);
	if (xldir == NULL)
		pg_fatal("could not open directory \"%s\": %m", WALSUMMARYDIR);
        ...

	if (errno)
		pg_fatal("could not read directory \"%s\": %m", WALSUMMARYDIR);

	if (closedir(xldir))
		pg_fatal("could not close directory \"%s\": %m", ARCHSTATDIR); <<=== It should really be WALSUMMARYDIR
}
```

I guess closedir() is hard to fail, that’s why the problem has not been noticed earlier.

The patch is straightforward and looks correct.

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

  4. Re: pg_resetwal: Fix wrong directory in log output

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-02-03T02:25:41Z

    Hello.

At Tue, 3 Feb 2026 10:13:53 +0800, Chao Li <li.evan.chao@gmail.com> wrote in 
> The patch is straightforward and looks correct.

Indeed, it does.

However, the scoping of the define symbols in this file seems somewhat
confused.

For example:

>   static void
>   KillExistingArchiveStatus(void)
>   {
>   #define ARCHSTATDIR XLOGDIR "/archive_status"

This define was presumably intended to be used only within the function,
but since it is a macro, it can also be used from other functions, which
appears to have contributed to this mistake.

It would be better to clearly distinguish between file-scoped symbols
and function-scoped ones. For function-local constants like this,
using a 'static const char[] instead of a macro would make the
intended scope explicit and help prevent similar issues.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  5. Re: pg_resetwal: Fix wrong directory in log output

    Chao Li <li.evan.chao@gmail.com> — 2026-02-03T03:46:03Z

    
> On Feb 3, 2026, at 10:25, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> 
> Hello.
> 
> At Tue, 3 Feb 2026 10:13:53 +0800, Chao Li <li.evan.chao@gmail.com> wrote in
>> The patch is straightforward and looks correct.
> 
> Indeed, it does.
> 
> However, the scoping of the define symbols in this file seems somewhat
> confused.
> 
> For example:
> 
>>  static void
>>  KillExistingArchiveStatus(void)
>>  {
>>  #define ARCHSTATDIR XLOGDIR "/archive_status"
> 
> This define was presumably intended to be used only within the function,
> but since it is a macro, it can also be used from other functions, which
> appears to have contributed to this mistake.
> 
> It would be better to clearly distinguish between file-scoped symbols
> and function-scoped ones. For function-local constants like this,
> using a 'static const char[] instead of a macro would make the
> intended scope explicit and help prevent similar issues.
> 

Or we can just undef the macros before closing the functions.

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

  6. Re: pg_resetwal: Fix wrong directory in log output

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-02-03T04:07:08Z

    At Tue, 3 Feb 2026 11:46:03 +0800, Chao Li <li.evan.chao@gmail.com> wrote in 
> Or we can just undef the macros before closing the functions.

Yeah, that’s also an option - we’ve seen it adopted elsewhere.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  7. Re:Re: pg_resetwal: Fix wrong directory in log output

    Tianchen Zhang <zhang_tian_chen@163.com> — 2026-02-03T06:16:56Z

    At 2026-02-03 10:13:53, "Chao Li" <li.evan.chao@gmail.com> wrote:
>
>
>> On Feb 3, 2026, at 09:58, zhang_tian_chen@163.com wrote:
>> 
>> Hi hackers,
>> 
>> There is a misuse of macro when we output directory information in KillExistingWALSummaries(), pg_resetwal.c. Which should be WALSUMMARYDIR instead of ARCHSTATDIR.
>> 
>> Best regards,
>> Tianchen Zhang<v1-0001-Fix-incorrect-directory-macro-in-KillExistingWALS.patch>
>
>Indeed a bug. Looking at the code:
>
>```
>/*
> * Remove existing WAL summary files
> */
>static void
>KillExistingWALSummaries(void)
>{
>	DIR		   *xldir;
>
>	xldir = opendir(WALSUMMARYDIR);
>	if (xldir == NULL)
>		pg_fatal("could not open directory \"%s\": %m", WALSUMMARYDIR);
>        ...
>
>	if (errno)
>		pg_fatal("could not read directory \"%s\": %m", WALSUMMARYDIR);
>
>	if (closedir(xldir))
>		pg_fatal("could not close directory \"%s\": %m", ARCHSTATDIR); <<=== It should really be WALSUMMARYDIR
>}
>```
>
>I guess closedir() is hard to fail, that’s why the problem has not been noticed earlier.
>
>The patch is straightforward and looks correct.
>
>Best regards,
>--
>Chao Li (Evan)
>HighGo Software Co., Ltd.
>https://www.highgo.com/
>
>

As the discussion above, I've updated the patch to v2 that also undefines the function-scoped marco at the end of corresponding fuctions.

Best regards,
Tianchen Zhang

  8. Re: pg_resetwal: Fix wrong directory in log output

    Chao Li <li.evan.chao@gmail.com> — 2026-02-03T06:28:22Z

    
> On Feb 3, 2026, at 14:16, Tianchen Zhang <zhang_tian_chen@163.com> wrote:
> 
> 
> At 2026-02-03 10:13:53, "Chao Li" <li.evan.chao@gmail.com> wrote:
>> 
>> 
>>> On Feb 3, 2026, at 09:58, zhang_tian_chen@163.com wrote:
>>> 
>>> Hi hackers,
>>> 
>>> There is a misuse of macro when we output directory information in KillExistingWALSummaries(), pg_resetwal.c. Which should be WALSUMMARYDIR instead of ARCHSTATDIR.
>>> 
>>> Best regards,
>>> Tianchen Zhang<v1-0001-Fix-incorrect-directory-macro-in-KillExistingWALS.patch>
>> 
>> Indeed a bug. Looking at the code:
>> 
>> ```
>> /*
>> * Remove existing WAL summary files
>> */
>> static void
>> KillExistingWALSummaries(void)
>> {
>> DIR   *xldir;
>> 
>> xldir = opendir(WALSUMMARYDIR);
>> if (xldir == NULL)
>> pg_fatal("could not open directory \"%s\": %m", WALSUMMARYDIR);
>>       ...
>> 
>> if (errno)
>> pg_fatal("could not read directory \"%s\": %m", WALSUMMARYDIR);
>> 
>> if (closedir(xldir))
>> pg_fatal("could not close directory \"%s\": %m", ARCHSTATDIR); <<=== It should really be WALSUMMARYDIR
>> }
>> ```
>> 
>> I guess closedir() is hard to fail, that’s why the problem has not been noticed earlier.
>> 
>> The patch is straightforward and looks correct.
>> 
>> Best regards,
>> --
>> Chao Li (Evan)
>> HighGo Software Co., Ltd.
>> https://www.highgo.com/
>> 
>> 
> 
> As the discussion above, I've updated the patch to v2 that also undefines the function-scoped marco at the end of corresponding fuctions.
> 
> Best regards,
> Tianchen Zhang
> <v2-0001-Fix-incorrect-directory-macro-in-KillExistingWALS.patch>

Thanks for updating the patch.

I applied v2 locally and the build passed. I also tried to revert to the wrong macro, now the compiler caught the mistake:
```
pg_resetwal.c:1116:52: error: use of undeclared identifier 'ARCHSTATDIR'
 1116 |                 pg_fatal("could not close directory \"%s\": %m", ARCHSTATDIR);
      |                                                                  ^
1 error generated.
make[3]: *** [pg_resetwal.o] Error 1
```

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

  9. Re: pg_resetwal: Fix wrong directory in log output

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-02-03T06:49:47Z

    At Tue, 3 Feb 2026 14:16:56 +0800 (CST), "Tianchen Zhang" <zhang_tian_chen@163.com> wrote in 
> As the discussion above, I've updated the patch to v2 that also undefines the function-scoped marco at the end of corresponding fuctions.

Thanks, looks good to me.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  10. Re: pg_resetwal: Fix wrong directory in log output

    Michael Paquier <michael@paquier.xyz> — 2026-02-04T07:39:35Z

    On Tue, Feb 03, 2026 at 03:49:47PM +0900, Kyotaro Horiguchi wrote:
> Thanks, looks good to me.

Yep, let's fix that.  Done.
--
Michael