agora inbox for pgsql-hackers@postgresql.orghelp / color / mirror / Atom feed
[PATCH v2 1/1] Remove durable_rename_excl(). 6+ messages / 2 participants [nested] [flat]
* [PATCH v2 1/1] Remove durable_rename_excl(). @ 2022-04-08 18:48 Nathan Bossart <nathandbossart@gmail.com> 0 siblings, 0 replies; 6+ messages in thread From: Nathan Bossart @ 2022-04-08 18:48 UTC (permalink / raw) durable_rename_excl() attempts to avoid overwriting any existing files by using link() and unlink(), but it falls back to rename() on some platforms (e.g., Windows), which offers no such ovewrite protection. Most callers use durable_rename_excl() just in case there is an existing file, but in practice there shouldn't be one. basic_archive uses it to avoid overwriting an archive concurrently created by another server, but as mentioned above, it will still overwrite files on some platforms. Furthermore, failures during durable_rename_excl() can result in multiple hard links to the same file. My testing demonstrated that it was possible to end up with two links to the same file in pg_wal after a crash just before unlink() during WAL recycling. Specifically, the test produced links to the same file for the current WAL file and the next one because the half-recycled WAL file was re-recycled upon restarting. This seems likely to lead to WAL corruption. This change removes durable_rename_excl() and replaces all existing calls with durable_rename(). This removes the protection against accidentally overwriting an existing file, but some platforms are already living without it, and ordinarily there shouldn't be one. Author: Nathan Bossart Reviewed-by: Robert Haas Discussion: https://postgr.es/m/20220407182954.GA1231544%40nathanxps13 --- contrib/basic_archive/basic_archive.c | 5 ++- src/backend/access/transam/timeline.c | 14 +----- src/backend/access/transam/xlog.c | 8 +--- src/backend/storage/file/fd.c | 63 --------------------------- src/include/pg_config_manual.h | 7 --- src/include/storage/fd.h | 1 - 6 files changed, 7 insertions(+), 91 deletions(-) diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c index e7efbfb9c3..ed33854c57 100644 --- a/contrib/basic_archive/basic_archive.c +++ b/contrib/basic_archive/basic_archive.c @@ -281,9 +281,10 @@ basic_archive_file_internal(const char *file, const char *path) /* * Sync the temporary file to disk and move it to its final destination. - * This will fail if destination already exists. + * Note that this will overwrite any existing file, but this is only + * possible if someone else created the file since the stat() above. */ - (void) durable_rename_excl(temp, destination, ERROR); + (void) durable_rename(temp, destination, ERROR); ereport(DEBUG1, (errmsg("archived \"%s\" via basic_archive", file))); diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c index be21968293..128f754e87 100644 --- a/src/backend/access/transam/timeline.c +++ b/src/backend/access/transam/timeline.c @@ -441,12 +441,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, * Now move the completed history file into place with its final name. */ TLHistoryFilePath(path, newTLI); - - /* - * Perform the rename using link if available, paranoidly trying to avoid - * overwriting an existing file (there shouldn't be one). - */ - durable_rename_excl(tmppath, path, ERROR); + durable_rename(tmppath, path, ERROR); /* The history file can be archived immediately. */ if (XLogArchivingActive()) @@ -519,12 +514,7 @@ writeTimeLineHistoryFile(TimeLineID tli, char *content, int size) * Now move the completed history file into place with its final name. */ TLHistoryFilePath(path, tli); - - /* - * Perform the rename using link if available, paranoidly trying to avoid - * overwriting an existing file (there shouldn't be one). - */ - durable_rename_excl(tmppath, path, ERROR); + durable_rename(tmppath, path, ERROR); } /* diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a7814d4019..d19215ab24 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -3323,14 +3323,10 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath, } } - /* - * Perform the rename using link if available, paranoidly trying to avoid - * overwriting an existing file (there shouldn't be one). - */ - if (durable_rename_excl(tmppath, path, LOG) != 0) + if (durable_rename(tmppath, path, LOG) != 0) { LWLockRelease(ControlFileLock); - /* durable_rename_excl already emitted log message */ + /* durable_rename already emitted log message */ return false; } diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 14b77f2861..88645ed83d 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -807,69 +807,6 @@ durable_unlink(const char *fname, int elevel) return 0; } -/* - * durable_rename_excl -- rename a file in a durable manner. - * - * Similar to durable_rename(), except that this routine tries (but does not - * guarantee) not to overwrite the target file. - * - * Note that a crash in an unfortunate moment can leave you with two links to - * the target file. - * - * Log errors with the caller specified severity. - * - * On Windows, using a hard link followed by unlink() causes concurrency - * issues, while a simple rename() does not cause that, so be careful when - * changing the logic of this routine. - * - * Returns 0 if the operation succeeded, -1 otherwise. Note that errno is not - * valid upon return. - */ -int -durable_rename_excl(const char *oldfile, const char *newfile, int elevel) -{ - /* - * Ensure that, if we crash directly after the rename/link, a file with - * valid contents is moved into place. - */ - if (fsync_fname_ext(oldfile, false, false, elevel) != 0) - return -1; - -#ifdef HAVE_WORKING_LINK - if (link(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not link file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } - unlink(oldfile); -#else - if (rename(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not rename file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } -#endif - - /* - * Make change persistent in case of an OS crash, both the new entry and - * its parent directory need to be flushed. - */ - if (fsync_fname_ext(newfile, false, false, elevel) != 0) - return -1; - - /* Same for parent directory */ - if (fsync_parent_path(newfile, elevel) != 0) - return -1; - - return 0; -} - /* * InitFileAccess --- initialize this module during backend startup * diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 84ce5a4a5d..830804fdfb 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -163,13 +163,6 @@ #define USE_BARRIER_SMGRRELEASE #endif -/* - * Define this if your operating system supports link() - */ -#if !defined(WIN32) && !defined(__CYGWIN__) -#define HAVE_WORKING_LINK 1 -#endif - /* * USE_POSIX_FADVISE controls whether Postgres will attempt to use the * posix_fadvise() kernel call. Usually the automatic configure tests are diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 69549b000f..2b4a8e0ffe 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -187,7 +187,6 @@ extern void fsync_fname(const char *fname, bool isdir); extern int fsync_fname_ext(const char *fname, bool isdir, bool ignore_perm, int elevel); extern int durable_rename(const char *oldfile, const char *newfile, int loglevel); extern int durable_unlink(const char *fname, int loglevel); -extern int durable_rename_excl(const char *oldfile, const char *newfile, int loglevel); extern void SyncDataDirectory(void); extern int data_sync_elevel(int elevel); -- 2.25.1 --LQksG6bCIzRHxTLp-- ^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v5 2/2] Remove durable_rename_excl(). @ 2022-04-26 19:38 Nathan Bossart <nathandbossart@gmail.com> 0 siblings, 0 replies; 6+ messages in thread From: Nathan Bossart @ 2022-04-26 19:38 UTC (permalink / raw) A previous commit replaced all calls to this function with durable_rename(), but the function itself was not removed in back- branches since extensions may use it. This change removes the function from v16devel. Do not back-patch. Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220418182336.GA2298576%40nathanxps13 --- src/backend/storage/file/fd.c | 63 ---------------------------------- src/include/pg_config_manual.h | 7 ---- src/include/storage/fd.h | 1 - 3 files changed, 71 deletions(-) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 24704b6a02..f904f60c08 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -807,69 +807,6 @@ durable_unlink(const char *fname, int elevel) return 0; } -/* - * durable_rename_excl -- rename a file in a durable manner. - * - * Similar to durable_rename(), except that this routine tries (but does not - * guarantee) not to overwrite the target file. - * - * Note that a crash in an unfortunate moment can leave you with two links to - * the target file. - * - * Log errors with the caller specified severity. - * - * On Windows, using a hard link followed by unlink() causes concurrency - * issues, while a simple rename() does not cause that, so be careful when - * changing the logic of this routine. - * - * Returns 0 if the operation succeeded, -1 otherwise. Note that errno is not - * valid upon return. - */ -int -durable_rename_excl(const char *oldfile, const char *newfile, int elevel) -{ - /* - * Ensure that, if we crash directly after the rename/link, a file with - * valid contents is moved into place. - */ - if (fsync_fname_ext(oldfile, false, false, elevel) != 0) - return -1; - -#ifdef HAVE_WORKING_LINK - if (link(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not link file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } - unlink(oldfile); -#else - if (rename(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not rename file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } -#endif - - /* - * Make change persistent in case of an OS crash, both the new entry and - * its parent directory need to be flushed. - */ - if (fsync_fname_ext(newfile, false, false, elevel) != 0) - return -1; - - /* Same for parent directory */ - if (fsync_parent_path(newfile, elevel) != 0) - return -1; - - return 0; -} - /* * InitFileAccess --- initialize this module during backend startup * diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 84ce5a4a5d..830804fdfb 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -163,13 +163,6 @@ #define USE_BARRIER_SMGRRELEASE #endif -/* - * Define this if your operating system supports link() - */ -#if !defined(WIN32) && !defined(__CYGWIN__) -#define HAVE_WORKING_LINK 1 -#endif - /* * USE_POSIX_FADVISE controls whether Postgres will attempt to use the * posix_fadvise() kernel call. Usually the automatic configure tests are diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 69549b000f..2b4a8e0ffe 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -187,7 +187,6 @@ extern void fsync_fname(const char *fname, bool isdir); extern int fsync_fname_ext(const char *fname, bool isdir, bool ignore_perm, int elevel); extern int durable_rename(const char *oldfile, const char *newfile, int loglevel); extern int durable_unlink(const char *fname, int loglevel); -extern int durable_rename_excl(const char *oldfile, const char *newfile, int loglevel); extern void SyncDataDirectory(void); extern int data_sync_elevel(int elevel); -- 2.25.1 --ZGiS0Q5IWpPtfppv-- ^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v5 2/2] Remove durable_rename_excl(). @ 2022-04-26 19:38 Nathan Bossart <nathandbossart@gmail.com> 0 siblings, 0 replies; 6+ messages in thread From: Nathan Bossart @ 2022-04-26 19:38 UTC (permalink / raw) A previous commit replaced all calls to this function with durable_rename(), but the function itself was not removed in back- branches since extensions may use it. This change removes the function from v16devel. Do not back-patch. Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220418182336.GA2298576%40nathanxps13 --- src/backend/storage/file/fd.c | 63 ---------------------------------- src/include/pg_config_manual.h | 7 ---- src/include/storage/fd.h | 1 - 3 files changed, 71 deletions(-) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 24704b6a02..f904f60c08 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -807,69 +807,6 @@ durable_unlink(const char *fname, int elevel) return 0; } -/* - * durable_rename_excl -- rename a file in a durable manner. - * - * Similar to durable_rename(), except that this routine tries (but does not - * guarantee) not to overwrite the target file. - * - * Note that a crash in an unfortunate moment can leave you with two links to - * the target file. - * - * Log errors with the caller specified severity. - * - * On Windows, using a hard link followed by unlink() causes concurrency - * issues, while a simple rename() does not cause that, so be careful when - * changing the logic of this routine. - * - * Returns 0 if the operation succeeded, -1 otherwise. Note that errno is not - * valid upon return. - */ -int -durable_rename_excl(const char *oldfile, const char *newfile, int elevel) -{ - /* - * Ensure that, if we crash directly after the rename/link, a file with - * valid contents is moved into place. - */ - if (fsync_fname_ext(oldfile, false, false, elevel) != 0) - return -1; - -#ifdef HAVE_WORKING_LINK - if (link(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not link file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } - unlink(oldfile); -#else - if (rename(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not rename file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } -#endif - - /* - * Make change persistent in case of an OS crash, both the new entry and - * its parent directory need to be flushed. - */ - if (fsync_fname_ext(newfile, false, false, elevel) != 0) - return -1; - - /* Same for parent directory */ - if (fsync_parent_path(newfile, elevel) != 0) - return -1; - - return 0; -} - /* * InitFileAccess --- initialize this module during backend startup * diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 84ce5a4a5d..830804fdfb 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -163,13 +163,6 @@ #define USE_BARRIER_SMGRRELEASE #endif -/* - * Define this if your operating system supports link() - */ -#if !defined(WIN32) && !defined(__CYGWIN__) -#define HAVE_WORKING_LINK 1 -#endif - /* * USE_POSIX_FADVISE controls whether Postgres will attempt to use the * posix_fadvise() kernel call. Usually the automatic configure tests are diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 69549b000f..2b4a8e0ffe 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -187,7 +187,6 @@ extern void fsync_fname(const char *fname, bool isdir); extern int fsync_fname_ext(const char *fname, bool isdir, bool ignore_perm, int elevel); extern int durable_rename(const char *oldfile, const char *newfile, int loglevel); extern int durable_unlink(const char *fname, int loglevel); -extern int durable_rename_excl(const char *oldfile, const char *newfile, int loglevel); extern void SyncDataDirectory(void); extern int data_sync_elevel(int elevel); -- 2.25.1 --ZGiS0Q5IWpPtfppv-- ^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v3 2/2] Remove durable_rename_excl(). @ 2022-04-26 19:38 Nathan Bossart <nathandbossart@gmail.com> 0 siblings, 0 replies; 6+ messages in thread From: Nathan Bossart @ 2022-04-26 19:38 UTC (permalink / raw) A previous commit replaced all calls to this function with durable_rename(), but the function itself was not removed in back- branches since extensions may use it. This change removes the function from v16devel. Do not back-patch. Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220418182336.GA2298576%40nathanxps13 --- src/backend/storage/file/fd.c | 63 ---------------------------------- src/include/pg_config_manual.h | 7 ---- src/include/storage/fd.h | 1 - 3 files changed, 71 deletions(-) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 24704b6a02..f904f60c08 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -807,69 +807,6 @@ durable_unlink(const char *fname, int elevel) return 0; } -/* - * durable_rename_excl -- rename a file in a durable manner. - * - * Similar to durable_rename(), except that this routine tries (but does not - * guarantee) not to overwrite the target file. - * - * Note that a crash in an unfortunate moment can leave you with two links to - * the target file. - * - * Log errors with the caller specified severity. - * - * On Windows, using a hard link followed by unlink() causes concurrency - * issues, while a simple rename() does not cause that, so be careful when - * changing the logic of this routine. - * - * Returns 0 if the operation succeeded, -1 otherwise. Note that errno is not - * valid upon return. - */ -int -durable_rename_excl(const char *oldfile, const char *newfile, int elevel) -{ - /* - * Ensure that, if we crash directly after the rename/link, a file with - * valid contents is moved into place. - */ - if (fsync_fname_ext(oldfile, false, false, elevel) != 0) - return -1; - -#ifdef HAVE_WORKING_LINK - if (link(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not link file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } - unlink(oldfile); -#else - if (rename(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not rename file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } -#endif - - /* - * Make change persistent in case of an OS crash, both the new entry and - * its parent directory need to be flushed. - */ - if (fsync_fname_ext(newfile, false, false, elevel) != 0) - return -1; - - /* Same for parent directory */ - if (fsync_parent_path(newfile, elevel) != 0) - return -1; - - return 0; -} - /* * InitFileAccess --- initialize this module during backend startup * diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 84ce5a4a5d..830804fdfb 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -163,13 +163,6 @@ #define USE_BARRIER_SMGRRELEASE #endif -/* - * Define this if your operating system supports link() - */ -#if !defined(WIN32) && !defined(__CYGWIN__) -#define HAVE_WORKING_LINK 1 -#endif - /* * USE_POSIX_FADVISE controls whether Postgres will attempt to use the * posix_fadvise() kernel call. Usually the automatic configure tests are diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 69549b000f..2b4a8e0ffe 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -187,7 +187,6 @@ extern void fsync_fname(const char *fname, bool isdir); extern int fsync_fname_ext(const char *fname, bool isdir, bool ignore_perm, int elevel); extern int durable_rename(const char *oldfile, const char *newfile, int loglevel); extern int durable_unlink(const char *fname, int loglevel); -extern int durable_rename_excl(const char *oldfile, const char *newfile, int loglevel); extern void SyncDataDirectory(void); extern int data_sync_elevel(int elevel); -- 2.25.1 --liOOAslEiF7prFVr-- ^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v4 2/2] Remove durable_rename_excl(). @ 2022-04-26 19:38 Nathan Bossart <nathandbossart@gmail.com> 0 siblings, 0 replies; 6+ messages in thread From: Nathan Bossart @ 2022-04-26 19:38 UTC (permalink / raw) A previous commit replaced all calls to this function with durable_rename(), but the function itself was not removed in back- branches since extensions may use it. This change removes the function from v16devel. Do not back-patch. Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220418182336.GA2298576%40nathanxps13 --- src/backend/storage/file/fd.c | 63 ---------------------------------- src/include/pg_config_manual.h | 7 ---- src/include/storage/fd.h | 1 - 3 files changed, 71 deletions(-) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 24704b6a02..f904f60c08 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -807,69 +807,6 @@ durable_unlink(const char *fname, int elevel) return 0; } -/* - * durable_rename_excl -- rename a file in a durable manner. - * - * Similar to durable_rename(), except that this routine tries (but does not - * guarantee) not to overwrite the target file. - * - * Note that a crash in an unfortunate moment can leave you with two links to - * the target file. - * - * Log errors with the caller specified severity. - * - * On Windows, using a hard link followed by unlink() causes concurrency - * issues, while a simple rename() does not cause that, so be careful when - * changing the logic of this routine. - * - * Returns 0 if the operation succeeded, -1 otherwise. Note that errno is not - * valid upon return. - */ -int -durable_rename_excl(const char *oldfile, const char *newfile, int elevel) -{ - /* - * Ensure that, if we crash directly after the rename/link, a file with - * valid contents is moved into place. - */ - if (fsync_fname_ext(oldfile, false, false, elevel) != 0) - return -1; - -#ifdef HAVE_WORKING_LINK - if (link(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not link file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } - unlink(oldfile); -#else - if (rename(oldfile, newfile) < 0) - { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not rename file \"%s\" to \"%s\": %m", - oldfile, newfile))); - return -1; - } -#endif - - /* - * Make change persistent in case of an OS crash, both the new entry and - * its parent directory need to be flushed. - */ - if (fsync_fname_ext(newfile, false, false, elevel) != 0) - return -1; - - /* Same for parent directory */ - if (fsync_parent_path(newfile, elevel) != 0) - return -1; - - return 0; -} - /* * InitFileAccess --- initialize this module during backend startup * diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 84ce5a4a5d..830804fdfb 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -163,13 +163,6 @@ #define USE_BARRIER_SMGRRELEASE #endif -/* - * Define this if your operating system supports link() - */ -#if !defined(WIN32) && !defined(__CYGWIN__) -#define HAVE_WORKING_LINK 1 -#endif - /* * USE_POSIX_FADVISE controls whether Postgres will attempt to use the * posix_fadvise() kernel call. Usually the automatic configure tests are diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 69549b000f..2b4a8e0ffe 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -187,7 +187,6 @@ extern void fsync_fname(const char *fname, bool isdir); extern int fsync_fname_ext(const char *fname, bool isdir, bool ignore_perm, int elevel); extern int durable_rename(const char *oldfile, const char *newfile, int loglevel); extern int durable_unlink(const char *fname, int loglevel); -extern int durable_rename_excl(const char *oldfile, const char *newfile, int loglevel); extern void SyncDataDirectory(void); extern int data_sync_elevel(int elevel); -- 2.25.1 --k+w/mQv8wyuph6w0-- ^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v3 3/3] Replace matview WITH OLD DATA @ 2024-07-26 21:33 Erik Wienhold <ewie@ewie.name> 0 siblings, 0 replies; 6+ messages in thread From: Erik Wienhold @ 2024-07-26 21:33 UTC (permalink / raw) --- .../sgml/ref/create_materialized_view.sgml | 16 +++++++++-- src/backend/commands/createas.c | 26 +++++++++++------ src/backend/parser/gram.y | 16 +++++++++++ src/include/nodes/primnodes.h | 1 + src/test/regress/expected/matview.out | 28 +++++++++++++++++++ src/test/regress/sql/matview.sql | 15 ++++++++++ 6 files changed, 90 insertions(+), 12 deletions(-) diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml index b5a8e3441a..65633b8bfa 100644 --- a/doc/src/sgml/ref/create_materialized_view.sgml +++ b/doc/src/sgml/ref/create_materialized_view.sgml @@ -27,7 +27,7 @@ CREATE [ OR REPLACE ] MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_nam [ WITH ( <replaceable class="parameter">storage_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ] [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ] AS <replaceable>query</replaceable> - [ WITH [ NO ] DATA ] + [ WITH [ NO | OLD ] DATA ] </synopsis> </refsynopsisdiv> @@ -37,7 +37,8 @@ CREATE [ OR REPLACE ] MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_nam <para> <command>CREATE MATERIALIZED VIEW</command> defines a materialized view of a query. The query is executed and used to populate the view at the time - the command is issued (unless <command>WITH NO DATA</command> is used) and may be + the command is issued (unless <command>WITH NO DATA</command> or + <command>WITH OLD DATA</command> is used) and may be refreshed later using <command>REFRESH MATERIALIZED VIEW</command>. </para> @@ -160,7 +161,7 @@ CREATE [ OR REPLACE ] MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_nam </varlistentry> <varlistentry> - <term><literal>WITH [ NO ] DATA</literal></term> + <term><literal>WITH [ NO | OLD ] DATA</literal></term> <listitem> <para> This clause specifies whether or not the materialized view should be @@ -168,6 +169,15 @@ CREATE [ OR REPLACE ] MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_nam flagged as unscannable and cannot be queried until <command>REFRESH MATERIALIZED VIEW</command> is used. </para> + + <para> + The form <command>WITH OLD DATA</command> keeps the already stored data + when replacing an existing materialized view to keep it populated. For + newly created materialized views, this has the same effect as + <command>WITH DATA</command>. Use this form if you want to use + <command>REFRESH MATERIALIZED VIEW CONCURRENTLY</command> as it requires + a populated materialized view. + </para> </listitem> </varlistentry> diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index e4ed3748f9..96e7b81966 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -331,18 +331,26 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt, /* An existing materialized view can be replaced. */ if (is_matview && into->replace) { - RefreshMatViewStmt *refresh; - /* Change the relation to match the new query and other options. */ - (void) create_ctas_nodata(query->targetList, into); + address = create_ctas_nodata(query->targetList, into); - /* Refresh the materialized view with a fake statement. */ - refresh = makeNode(RefreshMatViewStmt); - refresh->relation = into->rel; - refresh->skipData = into->skipData; - refresh->concurrent = false; + /* + * Refresh the materialized view with a fake statement unless we + * must keep the old data. + */ + if (!into->keepData) + { + RefreshMatViewStmt *refresh; + + refresh = makeNode(RefreshMatViewStmt); + refresh->relation = into->rel; + refresh->skipData = into->skipData; + refresh->concurrent = false; + + address = ExecRefreshMatView(refresh, NULL, NULL); + } - return ExecRefreshMatView(refresh, NULL, NULL); + return address; } return InvalidObjectAddress; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 4b939ea7ca..e45ebb24f8 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -4808,6 +4808,22 @@ CreateMatViewStmt: $7->replace = true; $$ = (Node *) ctas; } + | CREATE OR REPLACE OptNoLog MATERIALIZED VIEW create_mv_target AS SelectStmt WITH OLD DATA_P + { + CreateTableAsStmt *ctas = makeNode(CreateTableAsStmt); + + ctas->query = $9; + ctas->into = $7; + ctas->objtype = OBJECT_MATVIEW; + ctas->is_select_into = false; + ctas->if_not_exists = false; + /* cram additional flags into the IntoClause */ + $7->rel->relpersistence = $4; + $7->skipData = false; + $7->keepData = true; + $7->replace = true; + $$ = (Node *) ctas; + } ; create_mv_target: diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 4b0ee5d10d..ae84cb522e 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -168,6 +168,7 @@ typedef struct IntoClause /* materialized view's SELECT query */ Node *viewQuery pg_node_attr(query_jumble_ignore); bool skipData; /* true for WITH NO DATA */ + bool keepData; /* true for WITH OLD DATA */ bool replace; /* replace existing matview? */ } IntoClause; diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out index cefd0d442c..47dfd88bff 100644 --- a/src/test/regress/expected/matview.out +++ b/src/test/regress/expected/matview.out @@ -751,6 +751,23 @@ SELECT * FROM mvtest_replace; 3 (1 row) +-- replace query but keep old data +CREATE OR REPLACE MATERIALIZED VIEW mvtest_replace AS + SELECT 5 AS a + WITH OLD DATA; +SELECT * FROM mvtest_replace; + a +--- + 3 +(1 row) + +REFRESH MATERIALIZED VIEW mvtest_replace; +SELECT * FROM mvtest_replace; + a +--- + 5 +(1 row) + -- add column CREATE OR REPLACE MATERIALIZED VIEW mvtest_replace AS SELECT 4 AS a, 1 b; @@ -905,3 +922,14 @@ ERROR: syntax error at or near "NOT" LINE 1: CREATE OR REPLACE MATERIALIZED VIEW IF NOT EXISTS mvtest_rep... ^ DROP MATERIALIZED VIEW mvtest_replace; +-- Create new matview WITH OLD DATA. This populates the new matview as if +-- WITH DATA had been specified. +CREATE OR REPLACE MATERIALIZED VIEW mvtest_replace AS + SELECT 17 AS a + WITH OLD DATA; +SELECT * FROM mvtest_replace; + a +---- + 17 +(1 row) + diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql index c12f0243c9..b268237c24 100644 --- a/src/test/regress/sql/matview.sql +++ b/src/test/regress/sql/matview.sql @@ -338,6 +338,14 @@ SELECT * FROM mvtest_replace; -- error: not populated REFRESH MATERIALIZED VIEW mvtest_replace; SELECT * FROM mvtest_replace; +-- replace query but keep old data +CREATE OR REPLACE MATERIALIZED VIEW mvtest_replace AS + SELECT 5 AS a + WITH OLD DATA; +SELECT * FROM mvtest_replace; +REFRESH MATERIALIZED VIEW mvtest_replace; +SELECT * FROM mvtest_replace; + -- add column CREATE OR REPLACE MATERIALIZED VIEW mvtest_replace AS SELECT 4 AS a, 1 b; @@ -422,3 +430,10 @@ CREATE OR REPLACE MATERIALIZED VIEW IF NOT EXISTS mvtest_replace AS SELECT 1 AS a; DROP MATERIALIZED VIEW mvtest_replace; + +-- Create new matview WITH OLD DATA. This populates the new matview as if +-- WITH DATA had been specified. +CREATE OR REPLACE MATERIALIZED VIEW mvtest_replace AS + SELECT 17 AS a + WITH OLD DATA; +SELECT * FROM mvtest_replace; -- 2.46.0 --muysnh7l3kienazz-- ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-07-26 21:33 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-04-08 18:48 [PATCH v2 1/1] Remove durable_rename_excl(). Nathan Bossart <nathandbossart@gmail.com> 2022-04-26 19:38 [PATCH v5 2/2] Remove durable_rename_excl(). Nathan Bossart <nathandbossart@gmail.com> 2022-04-26 19:38 [PATCH v5 2/2] Remove durable_rename_excl(). Nathan Bossart <nathandbossart@gmail.com> 2022-04-26 19:38 [PATCH v3 2/2] Remove durable_rename_excl(). Nathan Bossart <nathandbossart@gmail.com> 2022-04-26 19:38 [PATCH v4 2/2] Remove durable_rename_excl(). Nathan Bossart <nathandbossart@gmail.com> 2024-07-26 21:33 [PATCH v3 3/3] Replace matview WITH OLD DATA Erik Wienhold <ewie@ewie.name>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox