Thread
Commits
-
Fix incorrect errno in OpenWalSummaryFile()
- 3b15032dad50 17.8 landed
- 719aa13b5857 18.2 landed
- 213fec296f41 19 (unreleased) landed
-
Incorrect errno used in OpenWalSummaryFile()
Michael Paquier <michael@paquier.xyz> — 2026-02-02T03:54:26Z
Hi all, While looking at walsummary.c for a different thread, I have bumped into this code: OpenWalSummaryFile(WalSummaryFile *ws, bool missing_ok) [...] file = PathNameOpenFile(path, O_RDONLY); if (file < 0 && (errno != EEXIST || !missing_ok)) ereport(ERROR, (errcode_for_file_access(), errmsg("could not open file \"%s\": %m", path))); And it seems to me that this EEXIST should be an ENOENT? The top of the function also documents that we want to handle an error when a summary file does not exist. The comment makes sense to me, not the code. It's also worth noting that this function has two callers, both use missing_ok = false, meaning that the errno check does not really matter today. If someone plays with this code on HEAD or the back-branches and decides to introduce a missing_ok=true call, it could matter, so I'd rather not change this function signature. This issue has been mentioned here as well, I've just bumped into it independently a few hours ago: https://www.postgresql.org/message-id/tencent_1CA40FCC21C1C770712BC089@qq.com Regards, -- Michael -
Re: Incorrect errno used in OpenWalSummaryFile()
Chao Li <li.evan.chao@gmail.com> — 2026-02-02T04:07:57Z
> On Feb 2, 2026, at 11:54, Michael Paquier <michael@paquier.xyz> wrote: > > Hi all, > > While looking at walsummary.c for a different thread, I have bumped > into this code: > OpenWalSummaryFile(WalSummaryFile *ws, bool missing_ok) > [...] > file = PathNameOpenFile(path, O_RDONLY); > if (file < 0 && (errno != EEXIST || !missing_ok)) > ereport(ERROR, > (errcode_for_file_access(), > errmsg("could not open file \"%s\": %m", path))); > > And it seems to me that this EEXIST should be an ENOENT? The top of > the function also documents that we want to handle an error when a > summary file does not exist. The comment makes sense to me, not the > code. > > It's also worth noting that this function has two callers, both use > missing_ok = false, meaning that the errno check does not really > matter today. If someone plays with this code on HEAD or the > back-branches and decides to introduce a missing_ok=true call, it > could matter, so I'd rather not change this function signature. > > This issue has been mentioned here as well, I've just bumped into it > independently a few hours ago: > https://www.postgresql.org/message-id/tencent_1CA40FCC21C1C770712BC089@qq.com > > Regards, > -- > Michael > <wal-summary-errno.patch> I think this is a correct fix. ``` [EEXIST] O_CREAT and O_EXCL are specified and the file exists. [ENOENT] O_CREAT is not set and the named file does not exist. ``` Here, the open flag is only O_RDONLY, so we should only expect ENOENT. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/