Re: CREATE DATABASE with filesystem cloning - Mailing list pgsql-hackers

Hi Ranier,

Thanks for looking into this!

I am not sure why but your reply does not show up in the thread, so I
copied your reply and answered it in the thread for visibility.

On Tue, 7 May 2024 at 16:28, Ranier Vilela <ranier.vf@gmail.com> wrote:
>
> I know it's coming from copy-and-paste, but
> I believe the flags could be:
> - dstfd = OpenTransientFile(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY);
> + dstfd = OpenTransientFile(tofile, O_CREAT | O_WRONLY | O_TRUNC | O_EXCL | PG_BINARY);
>
> The flags:
> O_WRONLY | O_TRUNC
>
> Allow the OS to make some optimizations, if you deem it possible.
>
> The flag O_RDWR indicates that the file can be read, which is not true in this case.
> The destination file will just be written.

You may be right about the O_WRONLY flag but I am not sure about the
O_TRUNC flag.

O_TRUNC from the linux man page [1]: If the file already exists and is
a regular file and the access mode allows writing (i.e., is O_RDWR or
O_WRONLY) it will be truncated to length 0.  If the file is a FIFO  or
terminal device file, the O_TRUNC flag is ignored. Otherwise, the
effect of O_TRUNC is unspecified.

But it should be already checked if the destination directory already
exists in dbcommands.c file in createdb() function [2], so this should
not happen.

[1] https://man7.org/linux/man-pages/man2/open.2.html

[2]
    /*
     * If database OID is configured, check if the OID is already in use or
     * data directory already exists.
     */
    if (OidIsValid(dboid))
    {
        char       *existing_dbname = get_database_name(dboid);

        if (existing_dbname != NULL)
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
                    errmsg("database OID %u is already in use by
database \"%s\"",
                           dboid, existing_dbname));

        if (check_db_file_conflict(dboid))
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
                    errmsg("data directory with the specified OID %u
already exists", dboid));
    }
    else
    {
        /* Select an OID for the new database if is not explicitly
configured. */
        do
        {
            dboid = GetNewOidWithIndex(pg_database_rel, DatabaseOidIndexId,
                                       Anum_pg_database_oid);
        } while (check_db_file_conflict(dboid));
    }

-- 
Regards,
Nazir Bilal Yavuz
Microsoft