Re: Atomic rename feature for Windows.
Hi Updated patch: we use the posix semantic features in Windows build 17763 and up. We found an issue with this feature on Windows Server 2016 without updates (Windows 1607 Build 14393) Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 05.07.2021 16:53, Victor Spirin пишет: Hi I used the SetFileInformationByHandle function with the FILE_RENAME_FLAG_POSIX_SEMANTICS flag for the file rename function.. 1) The _WIN32_WINNT variable needs to be increased to 0x0A00 (Windows 10). Fixed conflict with #undef CHECKSUM_TYPE_NONE 2) The SetFileInformationByHandle function works correctly only on Windows 10 and higher. The app must have a manifest to check the Windows version using the IsWindows10OrGreater() function. I added a manifest to all postgres projects and disabled the GenerateManifest option on windows projects. This patch related to this post: https://www.postgresql.org/message-id/CAEepm%3D0FV-k%2B%3Dd9z08cW%3DZXoR1%3Dkw9wdpkP6WAuOrKJdz-8ujg%40mail.gmail.com diff --git a/src/include/common/checksum_helper.h b/src/include/common/checksum_helper.h index 23816cac21..59e819f9ca 100644 --- a/src/include/common/checksum_helper.h +++ b/src/include/common/checksum_helper.h @@ -26,6 +26,13 @@ * MD5 here because any new that does need a cryptographically strong checksum * should use something better. */ + + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif typedef enum pg_checksum_type { CHECKSUM_TYPE_NONE, diff --git a/src/include/port/win32.h b/src/include/port/win32.h index c6213c77c3..c48a6ce3a1 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -12,12 +12,13 @@ /* * Make sure _WIN32_WINNT has the minimum required value. * Leave a higher value in place. When building with at least Visual - * Studio 2015 the minimum requirement is Windows Vista (0x0600) to - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 7ce042e75d..199b285a8f 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -39,6 +39,200 @@ #endif #endif + +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +/* + * Checks Windows version using RtlGetVersion + * Version 1809 (Build 17763) is required for SetFileInformationByHandle + * function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag +*/ +typedef NTSYSAPI(NTAPI* PFN_RTLGETVERSION) +(OUT PRTL_OSVERSIONINFOEXW lpVersionInformation); + +static int windowsPosixSemanticsUsable = -1; + +static bool +is_windows_posix_semantics_usable() +{ + HMODULE ntdll; + + PFN_RTLGETVERSION _RtlGetVersion = NULL; + OSVERSIONINFOEXW info; + + if (windowsPosixSemanticsUsable >= 0) + return (windowsPosixSemanticsUsable > 0); + + ntdll = LoadLibraryEx("ntdll.dll", NULL, 0); + if (ntdll == NULL) + { + DWORD err = GetLastError(); + + _dosmaperr(err); + return false; + } + + _RtlGetVersion = (PFN_RTLGETVERSION)(pg_funcptr_t) + GetProcAddress(ntdll, "RtlGetVersion"); + if (_RtlGetVersion == NULL) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return false; + } + info.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEXW); + if (!NT_SUCCESS(_RtlGetVersion())) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return false; + } + + if (info.dwMajorVersion >= 10 && info.dwBuildNumber >= 17763) + windowsPosixSemanticsUsable = 1; + else + windowsPosixSemanticsUsable = 0; + FreeLibrary(ntdll); + + return (windowsPosixSemanticsUsable > 0); +} + +typedef struct _FILE_RENAME_INFO_EXT { + FILE_RENAME_INFO fri; + WCHAR extra_space[MAX_PATH]; +} FILE_RENAME_INFO_EXT; + +/* + * pgrename_windows_posix_semantics - uses SetFileInformationByHandle function + * with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 (1809) or later and _WIN32_WINNT must be >= _WIN32_WINNT_WIN10 + */ +static int
Re: Atomic rename feature for Windows.
Hi The flags for calling the CreateFile function have been changed. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 05.07.2021 16:53, Victor Spirin пишет: Hi I used the SetFileInformationByHandle function with the FILE_RENAME_FLAG_POSIX_SEMANTICS flag for the file rename function.. 1) The _WIN32_WINNT variable needs to be increased to 0x0A00 (Windows 10). Fixed conflict with #undef CHECKSUM_TYPE_NONE 2) The SetFileInformationByHandle function works correctly only on Windows 10 and higher. The app must have a manifest to check the Windows version using the IsWindows10OrGreater() function. I added a manifest to all postgres projects and disabled the GenerateManifest option on windows projects. This patch related to this post: https://www.postgresql.org/message-id/CAEepm%3D0FV-k%2B%3Dd9z08cW%3DZXoR1%3Dkw9wdpkP6WAuOrKJdz-8ujg%40mail.gmail.com diff --git a/src/include/common/checksum_helper.h b/src/include/common/checksum_helper.h index cac7570ea1..2d533c93a6 100644 --- a/src/include/common/checksum_helper.h +++ b/src/include/common/checksum_helper.h @@ -26,6 +26,13 @@ * MD5 here because any new that does need a cryptographically strong checksum * should use something better. */ + + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif typedef enum pg_checksum_type { CHECKSUM_TYPE_NONE, diff --git a/src/include/port/win32.h b/src/include/port/win32.h index c6213c77c3..c48a6ce3a1 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -12,12 +12,13 @@ /* * Make sure _WIN32_WINNT has the minimum required value. * Leave a higher value in place. When building with at least Visual - * Studio 2015 the minimum requirement is Windows Vista (0x0600) to - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 763bc5f915..031655f0c8 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -39,6 +39,192 @@ #endif #endif + +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +/* + * Checks Windows version using RtlGetVersion + * Version 1607 (Build 14393) is required for SetFileInformationByHandle + * function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag +*/ +typedef NTSYSAPI(NTAPI * PFN_RTLGETVERSION) +(OUT PRTL_OSVERSIONINFOEXW lpVersionInformation); + +static int isWin1607 = -1; +static int isWindows1607OrGreater() +{ + HMODULE ntdll; + PFN_RTLGETVERSION _RtlGetVersion = NULL; + OSVERSIONINFOEXW info; + if (isWin1607 >= 0) return isWin1607; + ntdll = LoadLibraryEx("ntdll.dll", NULL, 0); + if (ntdll == NULL) + { + DWORD err = GetLastError(); + + _dosmaperr(err); + return -1; + } + + _RtlGetVersion = (PFN_RTLGETVERSION)(pg_funcptr_t) + GetProcAddress(ntdll, "RtlGetVersion"); + if (_RtlGetVersion == NULL) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return -1; + } + info.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEXW); + if (!NT_SUCCESS(_RtlGetVersion())) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return -1; + } + + if (info.dwMajorVersion >= 10 && info.dwBuildNumber >= 14393) isWin1607 = 1; + else isWin1607 = 0; + FreeLibrary(ntdll); + return isWin1607; + +} + +typedef struct _FILE_RENAME_INFO_EXT { + FILE_RENAME_INFO fri; + WCHAR extra_space[MAX_PATH]; +} FILE_RENAME_INFO_EXT; + +/* + * pgrename_windows_posix_semantics - uses SetFileInformationByHandle function + * with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 (1607) or later and _WIN32_WINNT must be >= _WIN32_WINNT_WIN10 + */ +static int +pgrename_windows_posix_semantics(const char *from, const char *to) +{ + int err; + FILE_RENAME_INFO_EXT rename_info; + PFILE_RENAME_INFO prename_info; + HANDLE f_handle; + wchar_t from_w[MAX_PATH]; + + prename_info = (PFILE_RENAME_INFO)_info; + + if (MultiByteToWideChar(CP_ACP, 0, (LPCC
Re: Atomic rename feature for Windows.
Hi Added the pgunlink_windows_posix_semantics function and modified the pgunlink function I used FILE_DISPOSITION_POSIX_SEMANTICS flag for unlink files on Windows 10 (1607) and above. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 05.07.2021 16:53, Victor Spirin пишет: Hi I used the SetFileInformationByHandle function with the FILE_RENAME_FLAG_POSIX_SEMANTICS flag for the file rename function.. 1) The _WIN32_WINNT variable needs to be increased to 0x0A00 (Windows 10). Fixed conflict with #undef CHECKSUM_TYPE_NONE 2) The SetFileInformationByHandle function works correctly only on Windows 10 and higher. The app must have a manifest to check the Windows version using the IsWindows10OrGreater() function. I added a manifest to all postgres projects and disabled the GenerateManifest option on windows projects. This patch related to this post: https://www.postgresql.org/message-id/CAEepm%3D0FV-k%2B%3Dd9z08cW%3DZXoR1%3Dkw9wdpkP6WAuOrKJdz-8ujg%40mail.gmail.com diff --git a/src/include/common/checksum_helper.h b/src/include/common/checksum_helper.h index cac7570ea1..2d533c93a6 100644 --- a/src/include/common/checksum_helper.h +++ b/src/include/common/checksum_helper.h @@ -26,6 +26,13 @@ * MD5 here because any new that does need a cryptographically strong checksum * should use something better. */ + + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif typedef enum pg_checksum_type { CHECKSUM_TYPE_NONE, diff --git a/src/include/port/win32.h b/src/include/port/win32.h index d8ae49e22d..d91555f5c0 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -12,12 +12,13 @@ /* * Make sure _WIN32_WINNT has the minimum required value. * Leave a higher value in place. When building with at least Visual - * Studio 2015 the minimum requirement is Windows Vista (0x0600) to - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 763bc5f915..6798b83908 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -39,6 +39,197 @@ #endif #endif + +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +/* + * Checks Windows version using RtlGetVersion + * Version 1607 (Build 14393) is required for SetFileInformationByHandle + * function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag +*/ +typedef NTSYSAPI(NTAPI * PFN_RTLGETVERSION) +(OUT PRTL_OSVERSIONINFOEXW lpVersionInformation); + +static int isWin1607 = -1; +static int isWindows1607OrGreater() +{ + HMODULE ntdll; + PFN_RTLGETVERSION _RtlGetVersion = NULL; + OSVERSIONINFOEXW info; + if (isWin1607 >= 0) return isWin1607; + ntdll = LoadLibraryEx("ntdll.dll", NULL, 0); + if (ntdll == NULL) + { + DWORD err = GetLastError(); + + _dosmaperr(err); + return -1; + } + + _RtlGetVersion = (PFN_RTLGETVERSION)(pg_funcptr_t) + GetProcAddress(ntdll, "RtlGetVersion"); + if (_RtlGetVersion == NULL) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return -1; + } + info.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEXW); + if (!NT_SUCCESS(_RtlGetVersion())) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return -1; + } + + if (info.dwMajorVersion >= 10 && info.dwBuildNumber >= 14393) isWin1607 = 1; + else isWin1607 = 0; + FreeLibrary(ntdll); + return isWin1607; + +} + +typedef struct _FILE_RENAME_INFO_EXT { + FILE_RENAME_INFO fri; + WCHAR extra_space[MAX_PATH]; +} FILE_RENAME_INFO_EXT; + +/* + * pgrename_windows_posix_semantics - uses SetFileInformationByHandle function + * with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 (1607) or later and _WIN32_WINNT must be >= _WIN32_WINNT_WIN10 + */ +static int +pgrename_windows_posix_semantics(const char *from, const char *to) +{ + int err; + FILE_RENAME_INFO_EXT rename_info; + PFILE_RENAME_INFO prename_info; + HANDLE f_handle; + wch
Re: Atomic rename feature for Windows.
Hi Added a small fix for calling the GetFileAttributes function Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 05.07.2021 16:53, Victor Spirin пишет: Hi I used the SetFileInformationByHandle function with the FILE_RENAME_FLAG_POSIX_SEMANTICS flag for the file rename function.. 1) The _WIN32_WINNT variable needs to be increased to 0x0A00 (Windows 10). Fixed conflict with #undef CHECKSUM_TYPE_NONE 2) The SetFileInformationByHandle function works correctly only on Windows 10 and higher. The app must have a manifest to check the Windows version using the IsWindows10OrGreater() function. I added a manifest to all postgres projects and disabled the GenerateManifest option on windows projects. This patch related to this post: https://www.postgresql.org/message-id/CAEepm%3D0FV-k%2B%3Dd9z08cW%3DZXoR1%3Dkw9wdpkP6WAuOrKJdz-8ujg%40mail.gmail.com diff --git a/src/include/common/checksum_helper.h b/src/include/common/checksum_helper.h index cac7570ea1..2d533c93a6 100644 --- a/src/include/common/checksum_helper.h +++ b/src/include/common/checksum_helper.h @@ -26,6 +26,13 @@ * MD5 here because any new that does need a cryptographically strong checksum * should use something better. */ + + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif typedef enum pg_checksum_type { CHECKSUM_TYPE_NONE, diff --git a/src/include/port/win32.h b/src/include/port/win32.h index d8ae49e22d..d91555f5c0 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -12,12 +12,13 @@ /* * Make sure _WIN32_WINNT has the minimum required value. * Leave a higher value in place. When building with at least Visual - * Studio 2015 the minimum requirement is Windows Vista (0x0600) to - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 763bc5f915..12e9c35cef 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -39,6 +39,146 @@ #endif #endif + +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +/* + * Checks Windows version using RtlGetVersion + * Version 1607 (Build 14393) is required for SetFileInformationByHandle + * function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag +*/ +typedef NTSYSAPI(NTAPI * PFN_RTLGETVERSION) +(OUT PRTL_OSVERSIONINFOEXW lpVersionInformation); + +static int isWin1607 = -1; +static int isWindows1607OrGreater() +{ + HMODULE ntdll; + PFN_RTLGETVERSION _RtlGetVersion = NULL; + OSVERSIONINFOEXW info; + if (isWin1607 >= 0) return isWin1607; + ntdll = LoadLibraryEx("ntdll.dll", NULL, 0); + if (ntdll == NULL) + { + DWORD err = GetLastError(); + + _dosmaperr(err); + return -1; + } + + _RtlGetVersion = (PFN_RTLGETVERSION)(pg_funcptr_t) + GetProcAddress(ntdll, "RtlGetVersion"); + if (_RtlGetVersion == NULL) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return -1; + } + info.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEXW); + if (!NT_SUCCESS(_RtlGetVersion())) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return -1; + } + + if (info.dwMajorVersion >= 10 && info.dwBuildNumber >= 14393) isWin1607 = 1; + else isWin1607 = 0; + FreeLibrary(ntdll); + return isWin1607; + +} + +typedef struct _FILE_RENAME_INFO_EXT { + FILE_RENAME_INFO fri; + WCHAR extra_space[MAX_PATH]; +} FILE_RENAME_INFO_EXT; + +/* + * pgrename_windows_posix_semantics - uses SetFileInformationByHandle function + * with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 (1607) or later and _WIN32_WINNT must be >= _WIN32_WINNT_WIN10 + */ +static int +pgrename_windows_posix_semantics(const char *from, const char *to) +{ + int err; + FILE_RENAME_INFO_EXT rename_info; + PFILE_RENAME_INFO prename_info; + HANDLE f_handle; + wchar_t from_w[MAX_PATH]; + DWORD dwFlagsAndAttributes; + DWORD dwAttrib; + + + prename_info = (PFILE_RENAME_INFO)_i
Re: Atomic rename feature for Windows.
Thank you Thank you In this version of patch: 1. Made function isWindows1607OrGreater() without manifest 2. To open a directory using CreateFile, have to specify the FILE_FLAG_BACKUP_SEMANTICS flag as part of dwFlagsAndAttributes. Checks that file is a directory by the GetFileAttributes function. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 01.10.2021 15:37, Juan José Santamaría Flecha пишет: On Thu, Sep 30, 2021 at 11:00 PM Victor Spirin mailto:v.spi...@postgrespro.ru>> wrote: IsWindowsVersionOrGreater(10,0,1607) always returns false Only IsWindowsVersionOrGreater(10,0,0) is a valid call. (There are no service packs in Windows 10.) I haven't found a way to determine the Windows 10 release ID. The RtlGetVersion function returns dwBuildNumber = 19042 on my Windows. I heard that Microsoft does not support older versions of Windows 10 and requires a mandatory update. You can translate the BuildNumber to the ReleaseId, for 1607 it will be 14393 [1]. We might find pretty much anything in the wild, the safer the check the better. [1] https://en.wikipedia.org/wiki/Windows_10_version_history <https://en.wikipedia.org/wiki/Windows_10_version_history> Regards, Juan José Santamaría Flecha diff --git a/src/include/common/checksum_helper.h b/src/include/common/checksum_helper.h index cac7570ea1..2d533c93a6 100644 --- a/src/include/common/checksum_helper.h +++ b/src/include/common/checksum_helper.h @@ -26,6 +26,13 @@ * MD5 here because any new that does need a cryptographically strong checksum * should use something better. */ + + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif typedef enum pg_checksum_type { CHECKSUM_TYPE_NONE, diff --git a/src/include/port/win32.h b/src/include/port/win32.h index d8ae49e22d..d91555f5c0 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -12,12 +12,13 @@ /* * Make sure _WIN32_WINNT has the minimum required value. * Leave a higher value in place. When building with at least Visual - * Studio 2015 the minimum requirement is Windows Vista (0x0600) to - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 763bc5f915..12e9c35cef 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -39,6 +39,146 @@ #endif #endif + +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +/* + * Checks Windows version using RtlGetVersion + * Version 1607 (Build 14393) is required for SetFileInformationByHandle + * function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag +*/ +typedef NTSYSAPI(NTAPI * PFN_RTLGETVERSION) +(OUT PRTL_OSVERSIONINFOEXW lpVersionInformation); + +static int isWin1607 = -1; +static int isWindows1607OrGreater() +{ + HMODULE ntdll; + PFN_RTLGETVERSION _RtlGetVersion = NULL; + OSVERSIONINFOEXW info; + if (isWin1607 >= 0) return isWin1607; + ntdll = LoadLibraryEx("ntdll.dll", NULL, 0); + if (ntdll == NULL) + { + DWORD err = GetLastError(); + + _dosmaperr(err); + return -1; + } + + _RtlGetVersion = (PFN_RTLGETVERSION)(pg_funcptr_t) + GetProcAddress(ntdll, "RtlGetVersion"); + if (_RtlGetVersion == NULL) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return -1; + } + info.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEXW); + if (!NT_SUCCESS(_RtlGetVersion())) + { + DWORD err = GetLastError(); + + FreeLibrary(ntdll); + _dosmaperr(err); + return -1; + } + + if (info.dwMajorVersion >= 10 && info.dwBuildNumber >= 14393) isWin1607 = 1; + else isWin1607 = 0; + FreeLibrary(ntdll); + return isWin1607; + +} + +typedef struct _FILE_RENAME_INFO_EXT { + FILE_RENAME_INFO fri; + WCHAR extra_space[MAX_PATH]; +} FILE_RENAME_INFO_EXT; + +/* + * pgrename_windows_posix_semantics - uses SetFileInformationByHandle function + * with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 (1607) or later and _WIN
Re: Atomic rename feature for Windows.
Thanks. IsWindowsVersionOrGreater(10,0,1607) always returns false Only IsWindowsVersionOrGreater(10,0,0) is a valid call. (There are no service packs in Windows 10.) I haven't found a way to determine the Windows 10 release ID. The RtlGetVersion function returns dwBuildNumber = 19042 on my Windows. I heard that Microsoft does not support older versions of Windows 10 and requires a mandatory update. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 23.09.2021 14:18, Juan José Santamaría Flecha пишет: On Tue, Sep 7, 2021 at 11:40 PM Victor Spirin <mailto:v.spi...@postgrespro.ru>> wrote: I checked the pgrename_windows_posix_semantics() function on Windows 7. It returns error 87: Parameter is incorrect. Hence, it is necessary to check the Windows version and call the old pgrename function for old Windows. The FILE_RENAME_FLAGs are available starting from Windows 10 Release id 1607, NTDDI_WIN10_RS1. The check should be using something like IsWindowsVersionOrGreater(10, 0, 1607). Or you could test this using RtlGetVersion(), loading it from ntdll infrastructure coming from stat() patch [1], which doesn't need a manifest. [1] https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BoLqfBVJ_j3C03QgoshrX1KxYq0LB1vJV0OXPOcZZfhA%40mail.gmail.com#bfcc256e4eda369e369275f5b4e38185 <https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BoLqfBVJ_j3C03QgoshrX1KxYq0LB1vJV0OXPOcZZfhA%40mail.gmail.com#bfcc256e4eda369e369275f5b4e38185> Regards, Juan José Santamaría Flecha
Re: Atomic rename feature for Windows.
Thank you, Fixed FILE_RENAME_INFO structure I prepared 2 versions of the patch: 1) with manifest and IsWindows10OrGreater() function 2) without manifest and RtlGetVersion function from ntdll.dll What's better? Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 23.09.2021 14:46, Thomas Munro пишет: On Wed, Sep 8, 2021 at 9:40 AM Victor Spirin wrote: Is this code better? Maybe there is another correct method? Hmm, if we want to use the system header's struct definition, add some space for a path at the end, and avoid heap allocation, perhaps we could do something like: struct { FILE_RENAME_INFO fri; WCHAR extra_space[MAX_PATH]; } x; diff --git a/src/include/common/checksum_helper.h b/src/include/common/checksum_helper.h index cac7570ea1..2d533c93a6 100644 --- a/src/include/common/checksum_helper.h +++ b/src/include/common/checksum_helper.h @@ -26,6 +26,13 @@ * MD5 here because any new that does need a cryptographically strong checksum * should use something better. */ + + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif typedef enum pg_checksum_type { CHECKSUM_TYPE_NONE, diff --git a/src/include/port/win32.h b/src/include/port/win32.h index d8ae49e22d..d91555f5c0 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -12,12 +12,13 @@ /* * Make sure _WIN32_WINNT has the minimum required value. * Leave a higher value in place. When building with at least Visual - * Studio 2015 the minimum requirement is Windows Vista (0x0600) to - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 763bc5f915..548b450b41 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -39,6 +39,87 @@ #endif #endif + +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +typedef struct _FILE_RENAME_INFO_EXT { + FILE_RENAME_INFO fri; + WCHAR extra_space[MAX_PATH]; +} FILE_RENAME_INFO_EXT; + + +/* + * pgrename_windows_posix_semantics - uses SetFileInformationByHandle function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 or later and _WIN32_WINNT must be >= _WIN32_WINNT_WIN10 + */ +static int +pgrename_windows_posix_semantics(const char *from, const char *to) +{ + int err; + FILE_RENAME_INFO_EXT rename_info; + PFILE_RENAME_INFO prename_info; + HANDLE f_handle; + wchar_t from_w[MAX_PATH]; + + + prename_info = (PFILE_RENAME_INFO)_info; + + if (MultiByteToWideChar(CP_ACP, 0, (LPCCH)from, -1, (LPWSTR)from_w, MAX_PATH) == 0) { + err = GetLastError(); + _dosmaperr(err); + return -1; + } + if (MultiByteToWideChar(CP_ACP, 0, (LPCCH)to, -1, (LPWSTR)prename_info->FileName, MAX_PATH) == 0) { + err = GetLastError(); + _dosmaperr(err); + return -1; + } + + f_handle = CreateFileW(from_w, + GENERIC_READ | GENERIC_WRITE | DELETE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + NULL); + + + if (f_handle == INVALID_HANDLE_VALUE) + { + err = GetLastError(); + + _dosmaperr(err); + + return -1; + } + + + prename_info->ReplaceIfExists = TRUE; + prename_info->Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS | FILE_RENAME_FLAG_REPLACE_IF_EXISTS; + + prename_info->RootDirectory = NULL; + prename_info->FileNameLength = wcslen(prename_info->FileName); + + if (!SetFileInformationByHandle(f_handle, FileRenameInfoEx, prename_info, sizeof(FILE_RENAME_INFO_EXT))) + { + err = GetLastError(); + + _dosmaperr(err); + CloseHandle(f_handle); + + return -1; + } + + CloseHandle(f_handle); + return 0; + +} + +#endif /* #if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 */ + + #if defined(WIN32) || defined(__CYGWIN__) /* @@ -49,6 +130,16 @
Re: Atomic rename feature for Windows.
Thank you, In this variant: 1) renamed file win10.manifest to windows.manifest 2) renamed function pgrename_win10 to pgrename_windows_posix_semantics 3) Function pgrename returns result of pgrename_windows_posix_semantics function and not contiue run old version of function. 4) Added call GetLastError() after error MultiByteToWideChar fuction. +typedef struct _FILE_RENAME_INFO_VVS { + union { + BOOLEAN ReplaceIfExists; // FileRenameInfo + DWORD Flags; // FileRenameInfoEx + } DUMMYUNIONNAME; + HANDLE RootDirectory; + DWORD FileNameLength; + WCHAR FileName[MAX_PATH]; +} FILE_RENAME_INFO_VVS; Why can't we use a system header[2] for this? I have a dynamic memory allocation version in the first patch. len = wcslen(to_w); rename_info = (FILE_RENAME_INFO*)malloc(sizeof(FILE_RENAME_INFO) + (len + 1) * sizeof(wchar_t)); rename_info->ReplaceIfExists = TRUE; rename_info->Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS | FILE_RENAME_FLAG_REPLACE_IF_EXISTS; rename_info->RootDirectory = NULL; rename_info->FileNameLength = len; memcpy(rename_info->FileName, to_w, (len + 1) * sizeof(wchar_t)); Is this code better? Maybe there is another correct method? I checked the pgrename_windows_posix_semantics() function on Windows 7. It returns error 87: Parameter is incorrect. Hence, it is necessary to check the Windows version and call the old pgrename function for old Windows. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 07.09.2021 4:40, Thomas Munro пишет: On Tue, Sep 7, 2021 at 5:44 AM Victor Spirin wrote: I have changed the way I add the manifest to projects. I used the AdditionalManifestFiles option for a VS project. Hi Victor, Thanks for working on this! I wonder if POSIX-style rename is used automatically on recent Windows, based on the new clue that DeleteFile() has started defaulting to POSIX semantics[1] (maybe it would require ReplaceFile() instead of MoveFileEx(), but I have no idea.) If so, one question is whether we'd still want to do this explicit POSIX rename dance, or whether we should just wait a bit longer for it to happen automatically on all relevant systems (plus tweak to use ReplaceFile() if necessary). If not, we might want to do what you're proposing anyway, especially if ReplaceFile() is required, because its interface is weird (it only works if the other file exists?). Hmm, that alone would be a good reason to go with your plan regardless, and of course it would be good to see this fixed everywhere ASAP. We still have to answer that question for pgunlink(). I was contemplating that over in that other thread, because unlink() -> EACCES is blocking something I'm working on. I found a partial solution to that that works even on old and non-NTFS systems, and I was thinking that would be enough for now and we could just patiently wait until automatic POSIX semantics to arrives on all relevant systems as the real long term solution, so I didn't need to expend energy doing an intermediate explicit POSIX-mode wrapper like what you're proposing. But then it seems strange to make a different choice about that for rename() and unlink(). So... do you think it would make sense to extend your patch to cover unlink() too? It would be great to have a tool in the tree that tests directory entry semantics, called something like src/bin/pg_test_dirmod, so that it becomes very clear when POSIX semantics are being used. It could test various interesting unlink and rename scenarios through our wrappers (concurrent file handles, creating a new file with the name of the old one, unlinking the containing directory, ...). It could run on the build farm animals, and we could even ask people to run it when they report problems, to try to lift the fog of bizarro Windows file system semantics. How exactly does the function fail on a file system that doesn't support the new POSIX semantics? Assuming there is something like ENOSUPP to report "file system says no", do we want to keep trying every time, or remember that it doesn't work? I guess the answer may vary per mount point, which makes it hard to track when you only have an fd... If it fails because of a sharing violation, it seems strange that we immediately fall back to the old code to do the traditional (and horrible) sleep/retry loop. That means that in rare conditions we can still get the old behaviour that leads to problems, just because of a transient condition. Hmm. Would it make more sense to say: fall back to the traditional behaviour only for ENOSUPP (if there is such a thing), cope with transient sharing violations without giving up on POSIX semantics, and report all other failures immediately? I agree that the existing enum CHECKSUM_TYPE_NONE + friends should be renamed to something less collision-prone and more consistent with the name of the enum ("pg_checksum_type"
Re: Atomic rename feature for Windows.
Hello. I have changed the way I add the manifest to projects. I used the AdditionalManifestFiles option for a VS project. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 08.07.2021 1:32, Victor Spirin пишет: Thanks! In this version of the patch, calls to malloc have been removed. Hopefully MAX_PATH is long enough for filenames. How does that cope with durable_rename_excl() where rename() is used on Windows? The problems that 909b449 has somewhat "fixed" were annoying for the users as it prevented WAL segment recycling, so we need to be sure that this does not cause more harm. I tested this patch to resolve the error message "could not rename temporary statistics file "pg_stat_tmp/pgstat.tmp" to "pg_stat_tmp/pgstat.stat": Permission denied". (I have a patch option to rename a temporary file for statistics only.) + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif Okay. Should this be renamed separately then to avoid conflicts? Renaming CHECKSUM_TYPE_NONE in the checksum_helper.h is the best way to go. #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif This is a large bump for Studio >= 2015 I am afraid. That does not seem acceptable, as it means losing support for GetLocaleInfoEx() across older versions. It seems that the MIN_WINNT value 0x0600 or 0x0A00 does not affect the use of the GetLocaleInfoEx () function + # manifest with ms_compatibility:supportedOS tags for using IsWindows10OrGreater() function + print $o "\n1 24 \"src/port/win10.manifest\"\n"; + close($o); close($i); } diff --git a/src/port/win10.manifest b/src/port/win10.manifest new file mode 100644 It would be good to not require that. Those extra files make the long-term maintenance harder. Function IsWindows10OrGreater() working properly if there is manifest with Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}" /> "Applications not manifested for Windows 10 return false, even if the current operating system version is Windows 10." Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 06.07.2021 4:43, Michael Paquier пишет: On Mon, Jul 05, 2021 at 04:53:06PM +0300, Victor Spirin wrote: This patch related to this post: https://www.postgresql.org/message-id/CAEepm%3D0FV-k%2B%3Dd9z08cW%3DZXoR1%3Dkw9wdpkP6WAuOrKJdz-8ujg%40mail.gmail.com How does that cope with durable_rename_excl() where rename() is used on Windows? The problems that 909b449 has somewhat "fixed" were annoying for the users as it prevented WAL segment recycling, so we need to be sure that this does not cause more harm. + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif Okay. Should this be renamed separately then to avoid conflicts? - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif This is a large bump for Studio >= 2015 I am afraid. That does not seem acceptable, as it means losing support for GetLocaleInfoEx() across older versions. +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +/* + * win10_rename - uses SetFileInformationByHandle function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 or later and _WIN32_WINNT must be >= _WIN32_WINNT_WIN10 + */ +static int win10_rename(wchar_t const* from, wchar_t const* to) Having win10_rename(), a wrapper for pgrename_win10(), which is itself an extra wrapper for pgrename(), is confusing. Could you reduce the layers of functions here. At the end we just want an extra runtime option for pgrename(). Note that pgrename_win10() could be static to dirmod.c, and it seems to me that you just want a small function to do the path conversion anyway. It would be better to avoid using malloc() in those code paths as well, as the backend could finish by calling that. We should be able to remove the malloc()s with local variables large enough to hold those paths, no? + # manifest with ms_compatibility:supportedOS tags f
Re: Atomic rename feature for Windows.
Thanks! In this version of the patch, calls to malloc have been removed. Hopefully MAX_PATH is long enough for filenames. How does that cope with durable_rename_excl() where rename() is used on Windows? The problems that 909b449 has somewhat "fixed" were annoying for the users as it prevented WAL segment recycling, so we need to be sure that this does not cause more harm. I tested this patch to resolve the error message "could not rename temporary statistics file "pg_stat_tmp/pgstat.tmp" to "pg_stat_tmp/pgstat.stat": Permission denied". (I have a patch option to rename a temporary file for statistics only.) + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif Okay. Should this be renamed separately then to avoid conflicts? Renaming CHECKSUM_TYPE_NONE in the checksum_helper.h is the best way to go. #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif This is a large bump for Studio >= 2015 I am afraid. That does not seem acceptable, as it means losing support for GetLocaleInfoEx() across older versions. It seems that the MIN_WINNT value 0x0600 or 0x0A00 does not affect the use of the GetLocaleInfoEx () function + # manifest with ms_compatibility:supportedOS tags for using IsWindows10OrGreater() function + print $o "\n1 24 \"src/port/win10.manifest\"\n"; + close($o); close($i); } diff --git a/src/port/win10.manifest b/src/port/win10.manifest new file mode 100644 It would be good to not require that. Those extra files make the long-term maintenance harder. Function IsWindows10OrGreater() working properly if there is manifest with Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}" /> "Applications not manifested for Windows 10 return false, even if the current operating system version is Windows 10." Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 06.07.2021 4:43, Michael Paquier пишет: On Mon, Jul 05, 2021 at 04:53:06PM +0300, Victor Spirin wrote: This patch related to this post: https://www.postgresql.org/message-id/CAEepm%3D0FV-k%2B%3Dd9z08cW%3DZXoR1%3Dkw9wdpkP6WAuOrKJdz-8ujg%40mail.gmail.com How does that cope with durable_rename_excl() where rename() is used on Windows? The problems that 909b449 has somewhat "fixed" were annoying for the users as it prevented WAL segment recycling, so we need to be sure that this does not cause more harm. + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif Okay. Should this be renamed separately then to avoid conflicts? - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif This is a large bump for Studio >= 2015 I am afraid. That does not seem acceptable, as it means losing support for GetLocaleInfoEx() across older versions. +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +/* + * win10_rename - uses SetFileInformationByHandle function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 or later and _WIN32_WINNT must be >= _WIN32_WINNT_WIN10 + */ +static int win10_rename(wchar_t const* from, wchar_t const* to) Having win10_rename(), a wrapper for pgrename_win10(), which is itself an extra wrapper for pgrename(), is confusing. Could you reduce the layers of functions here. At the end we just want an extra runtime option for pgrename(). Note that pgrename_win10() could be static to dirmod.c, and it seems to me that you just want a small function to do the path conversion anyway. It would be better to avoid using malloc() in those code paths as well, as the backend could finish by calling that. We should be able to remove the malloc()s with local variables large enough to hold those paths, no? + # manifest with ms_compatibility:supportedOS tags for using IsWindows10OrGreater() function + print $o "\n1 24 \"src/port/win10.manifest\"\n"; + close($o); close($i); } diff --git a/src/port/win10.man
Atomic rename feature for Windows.
Hi I used the SetFileInformationByHandle function with the FILE_RENAME_FLAG_POSIX_SEMANTICS flag for the file rename function.. 1) The _WIN32_WINNT variable needs to be increased to 0x0A00 (Windows 10). Fixed conflict with #undef CHECKSUM_TYPE_NONE 2) The SetFileInformationByHandle function works correctly only on Windows 10 and higher. The app must have a manifest to check the Windows version using the IsWindows10OrGreater() function. I added a manifest to all postgres projects and disabled the GenerateManifest option on windows projects. This patch related to this post: https://www.postgresql.org/message-id/CAEepm%3D0FV-k%2B%3Dd9z08cW%3DZXoR1%3Dkw9wdpkP6WAuOrKJdz-8ujg%40mail.gmail.com -- Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company diff --git a/src/include/common/checksum_helper.h b/src/include/common/checksum_helper.h index cac7570ea1..2d533c93a6 100644 --- a/src/include/common/checksum_helper.h +++ b/src/include/common/checksum_helper.h @@ -26,6 +26,13 @@ * MD5 here because any new that does need a cryptographically strong checksum * should use something better. */ + + /* + * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >= _WIN32_WINNT_WIN10 + */ +#ifdef CHECKSUM_TYPE_NONE +#undef CHECKSUM_TYPE_NONE +#endif typedef enum pg_checksum_type { CHECKSUM_TYPE_NONE, diff --git a/src/include/port/win32.h b/src/include/port/win32.h index d8ae49e22d..d91555f5c0 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -12,12 +12,13 @@ /* * Make sure _WIN32_WINNT has the minimum required value. * Leave a higher value in place. When building with at least Visual - * Studio 2015 the minimum requirement is Windows Vista (0x0600) to - * get support for GetLocaleInfoEx() with locales. For everything else + * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support for SetFileInformationByHandle. + * The minimum requirement is Windows Vista (0x0600) get support for GetLocaleInfoEx() with locales. + * For everything else * the minimum version is Windows XP (0x0501). */ #if defined(_MSC_VER) && _MSC_VER >= 1900 -#define MIN_WINNT 0x0600 +#define MIN_WINNT 0x0A00 #else #define MIN_WINNT 0x0501 #endif diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 763bc5f915..fbd051699d 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -39,6 +39,110 @@ #endif #endif + +#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 + +#include + +/* + * win10_rename - uses SetFileInformationByHandle function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file + * working only on Windows 10 or later and _WIN32_WINNT must be >= _WIN32_WINNT_WIN10 + */ +static int win10_rename(wchar_t const* from, wchar_t const* to) +{ + + int err; + size_t len; + FILE_RENAME_INFO* rename_info; + HANDLE f_handle; + + f_handle = CreateFileW(from, + GENERIC_READ | GENERIC_WRITE | DELETE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + NULL); + + + if (f_handle == INVALID_HANDLE_VALUE) + { + err = GetLastError(); + _dosmaperr(err); + return -1; + } + + + len = wcslen(to); + rename_info = (FILE_RENAME_INFO*)malloc(sizeof(FILE_RENAME_INFO) + (len + 1) * sizeof(wchar_t)); + rename_info->ReplaceIfExists = TRUE; + rename_info->Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS | FILE_RENAME_FLAG_REPLACE_IF_EXISTS; + + rename_info->RootDirectory = NULL; + rename_info->FileNameLength = len; + memcpy(rename_info->FileName, to, (len + 1) * sizeof(wchar_t)); + if (!SetFileInformationByHandle(f_handle, FileRenameInfoEx, rename_info, sizeof(FILE_RENAME_INFO) + (len + 1) * sizeof(wchar_t))) + { + err = GetLastError(); + + _dosmaperr(err); + CloseHandle(f_handle); + free(rename_info); + return -1; + } + + CloseHandle(f_handle); + + return 0; +} + + +/* + * pgrename_win10 - converts arguments from Windows ANSI code page to wchar_t and calls win10_rename function +*/ +int +pgrename_win10(const char *from, const char *to) +{ + wchar_t *from_w, *to_w; + size_t size, wsize; + int wlen; + int ret; + + size = strlen(from) + 1; + wsize = size * sizeof(wchar_t); + + from_w = (wchar_t*)malloc(wsize); + wlen = MultiByteToWideChar(CP_ACP, 0, (LPCCH)from, (int)size, (LPWSTR)from_w, (int)size); + if (wlen == 0) + { + free(from_w); + return -1; + } + + size = strlen(to) + 1; + wsize = size * sizeof(wchar
Re: Sometimes the output to the stdout in Windows disappears
I only found that calling WSACleanup instead of PQfinish has the same effect. I don't see any big performance issues due to the extra fflush call in this place. I will be glad to find a more elegant solution. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 31.08.2020 21:31, Tom Lane пишет: Victor Spirin writes: Sometimes the output to stdout on Windows on multicore machines does not go through after connecting and disconnecting to the server using the PQconnectdbParams and PQfinish functions. I tested on 6 cores. Hm, why is this not Microsoft's bug to solve? I do wonder if this report is related to the intermittent ecpg failures we see on Windows machines, such as [1]. The details vary, but it's always a case of a .stdout file ending up empty when it should not be. I'd supposed though that it must be something specific to ecpg, since we never see anything like that anywhere but the ecpg tests. Even if you posit that libpq is doing something that somehow compromises stdio, that should affect psql-based tests too. I am attaching a patch and a script for testing. [ forced fflush in every snprintf call ] My goodness, that's a large hammer you're swinging. What effects has this kluge got on performance? While I think you may be on to something, this seems like a truly horrid way to "fix" it. We need to dig down further and understand what is actually happening. regards, tom lane [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory=2020-08-13%2022%3A15%3A05
Sometimes the output to the stdout in Windows disappears
Hi! Sometimes the output to stdout on Windows on multicore machines does not go through after connecting and disconnecting to the server using the PQconnectdbParams and PQfinish functions. I tested on 6 cores. First we appeared this in some pgbench tests. Then we found that this happens on any console program using PQconnectdbParams and PQfinish. I am attaching a patch and a script for testing. -- Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company diff --git a/src/port/snprintf.c b/src/port/snprintf.c index 83a8103..17dc505 100644 --- a/src/port/snprintf.c +++ b/src/port/snprintf.c @@ -313,6 +313,14 @@ flushbuffer(PrintfTarget *target) target->failed = true; } target->bufptr = target->bufstart; + /* +* Sometimes the output to stdout on Windows on multicore machines +* does not go through after connecting and disconnecting to the server +* using the PQconnectdbParams and PQfinish functions. + */ +#ifdef WIN32 + fflush(target->stream); +#endif } use IPC::Run; my $clopts = '-p 5432 -e --dbname=postgres -U postgres'; my $stdout; my $stderr; my @cmd = ('clusterdb', split /\s+/, $clopts); my $ch; for (my $i =0; $i < 2*4096; $i++) { $ch = IPC::Run::run(\@cmd, '>', \$stdout, '2>', \$stderr); if(not defined $stdout) { print($i.": not defined\n"); } if($stdout eq '') { print($i.": stdout is empty\n"); } if ( ($i % 100) == 0){ print("$i\n"); } }
Re: psql tab-complete
This patch resolved one problem in the tab-complete.c on MSVC. The VA_ARGS_NARGS macros now work correctly on Windows. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 25.10.2019 0:53, Victor Spirin пишет: Yes, I found, that VA_ARGS_NARGS(__ VA_ARGS__) macros always return 1 on Windows. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 25.10.2019 0:48, Tom Lane пишет: Victor Spirin writes: I found some problem with tab-complete in the 12 version. I checked this in the Windows. This change seems to break the case intended by the comment, ie given the context SELECT * FROM tablename WHERE we want to offer the columns of "tablename" as completions. I'd be the first to agree that that's completely lame, as there are any number of related cases it fails to cover ... but this patch isn't making it better. regards, tom lane diff --git a/src/include/c.h b/src/include/c.h index edc7822b0f..ed7099164b 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -238,6 +238,19 @@ * the call so that that is the appropriate one of the list of constants. * This idea is due to Laurent Deniau. */ +#ifdef _MSC_VER +#define EXPAND(args) args +#define VA_ARGS_NARGS(...) \ + VA_ARGS_NARGS_ EXPAND((__VA_ARGS__, \ + 63,62,61,60, \ + 59,58,57,56,55,54,53,52,51,50, \ + 49,48,47,46,45,44,43,42,41,40, \ + 39,38,37,36,35,34,33,32,31,30, \ + 29,28,27,26,25,24,23,22,21,20, \ + 19,18,17,16,15,14,13,12,11,10, \ + 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)) +#else + #define VA_ARGS_NARGS(...) \ VA_ARGS_NARGS_(__VA_ARGS__, \ 63,62,61,60, \ @@ -247,6 +260,8 @@ 29,28,27,26,25,24,23,22,21,20, \ 19,18,17,16,15,14,13,12,11,10, \ 9, 8, 7, 6, 5, 4, 3, 2, 1, 0) +#endif + #define VA_ARGS_NARGS_( \ _01,_02,_03,_04,_05,_06,_07,_08,_09,_10, \ _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \
Re: psql tab-complete
Yes, I found, that VA_ARGS_NARGS(__ VA_ARGS__) macros always return 1 on Windows. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 25.10.2019 0:48, Tom Lane пишет: Victor Spirin writes: I found some problem with tab-complete in the 12 version. I checked this in the Windows. This change seems to break the case intended by the comment, ie given the context SELECT * FROM tablename WHERE we want to offer the columns of "tablename" as completions. I'd be the first to agree that that's completely lame, as there are any number of related cases it fails to cover ... but this patch isn't making it better. regards, tom lane
Re: psql tab-complete
Sorry for wrong place and contents of my message. It seems that the VA_ARGS_NARGS (__ VA_ARGS__) macros always return 1 on Windows. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 24.10.2019 19:11, Victor Spirin пишет: I found some problem with tab-complete in the 12 version. I checked this in the Windows. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company
psql tab-complete
I found some problem with tab-complete in the 12 version. I checked this in the Windows. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index e00dbab5aa..5d7f24e57a 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -3577,7 +3577,7 @@ psql_completion(const char *text, int start, int end) /* WHERE */ /* Simple case of the word before the where being the table name */ - else if (TailMatches(MatchAny, "WHERE")) + else if (TailMatches("WHERE", MatchAny)) COMPLETE_WITH_ATTR(prev2_wd, ""); /* ... FROM ... */
Re: PostgreSQL does not start when max_files_per_process> 1200 on Windows 7
When max_files_per_process=1201 or more server not start. I tested and debugged this on Windows 7. On Windows 10 work all right. I found this take place after call function count_usable_fds(int max_to_probe, int *usable_fds, int *already_open) Error occured on the first call selres = select(nSockets, , NULL, NULL, ) from ServerLoop. Error 0xC142: "{DLL Initialization Failed} Initialization of the dynamic link library %hs failed. The process is terminating abnormally." I temporarily moved call set_max_safe_fds() to top of the PostmasterMain and error moved to ReadFile in function pipe_read_line(char *cmd, char *line, int maxsize): if (!ReadFile(childstdoutrddup, lineptr, maxsize - (lineptr - line), , NULL)) Repeat the error in a separate example did not work yet. Replacing dup for stdin on dup of file handle resolve this problem. Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company 05.09.2018 19:24, Andres Freund пишет: Hi, On 2018-09-05 13:06:06 +0300, Victor Spirin wrote: I have a bug in Windows 7 with max_files_per_process> 1200. Using dup (0) in the function count_usable_fds more than 1200 times (0 = stdin) causes further unpredictable errors with file operations. Could you expand a bit on this? Greetings, Andres Freund
PostgreSQL does not start when max_files_per_process> 1200 on Windows 7
Hi, I have a bug in Windows 7 with max_files_per_process> 1200. Using dup (0) in the function count_usable_fds more than 1200 times (0 = stdin) causes further unpredictable errors with file operations. When I open a real file and use its descriptor for the dup, no error occurs. In the patch I check the file postgresql.conf -- Victor Spirin Postgres Professional:http://www.postgrespro.com The Russian Postgres Company diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 8dd51f1..79054e5 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -828,6 +828,15 @@ count_usable_fds(int max_to_probe, int *usable_fds, int *already_open) ereport(WARNING, (errmsg("getrlimit failed: %m"))); #endif /* HAVE_GETRLIMIT */ + int fd_test = 0; +#ifdef WIN32 + /* +* we have error on Windows7 with max_files_per_process > 1200 when dup(0) - stdin +* make test on postgresql.conf file +*/ + fd_test = _open(ConfigFileName, _O_RDONLY); + if (fd_test < 0) fd_test = 0; /* if was error then = stdin */ +#endif /* dup until failure or probe limit reached */ for (;;) { @@ -843,7 +852,7 @@ count_usable_fds(int max_to_probe, int *usable_fds, int *already_open) break; #endif - thisfd = dup(0); + thisfd = dup(fd_test); if (thisfd < 0) { /* Expect EMFILE or ENFILE, else it's fishy */ @@ -869,7 +878,9 @@ count_usable_fds(int max_to_probe, int *usable_fds, int *already_open) /* release the files we opened */ for (j = 0; j < used; j++) close(fd[j]); - +#ifdef WIN32 + if (fd_test > 0) _close(fd_test); +#endif pfree(fd); /*