Hi, On Fri, 2 Feb 2024 at 03:11, Artur Zakirov <zaar...@gmail.com> wrote: > > Hi hackers, > > during reading the source code of new incremental backup functionality > I noticed that the following condition can by unintentional: > > /* > * For newer server versions, likewise create pg_wal/summaries > */ > if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_WAL_SUMMARIES) > { > ... > > if (pg_mkdir_p(summarydir, pg_dir_create_mode) != 0 && > errno != EEXIST) > pg_fatal("could not create directory \"%s\": %m", summarydir); > } > > This is from src/bin/pg_basebackup/pg_basebackup.c. > > Is the condition correct? Shouldn't it be ">=". Otherwise the function > will create "/summaries" only for older PostgreSQL versions.
You seem right, nice catch. Also, this change makes the check in snprintf(summarydir, sizeof(summarydir), "%s/%s/summaries", basedir, PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal"); redundant. PQserverVersion(conn) will always be higher than MINIMUM_VERSION_FOR_PG_WAL. -- Regards, Nazir Bilal Yavuz Microsoft