Thread

Commits

  1. Fix incorrect errno in OpenWalSummaryFile()

  1. 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
    
  2. 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/