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 <ms_compatibility:supportedOS 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 <versionhelpers.h>
+
+/*
+ * 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.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.
--
Michael
diff --git a/src/include/common/checksum_helper.h 
b/src/include/common/checksum_helper.h
index cac7570ea13..2d533c93a6f 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 d8ae49e22d1..d91555f5c09 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 763bc5f915a..61615324442 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -39,6 +39,81 @@
 #endif
 #endif
 
+
+#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) && 
_WIN32_WINNT >= _WIN32_WINNT_WIN10
+
+#include <versionhelpers.h>
+
+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;
+
+/*
+ * pgrename_win10  - 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_win10(const char *from, const char *to)
+{
+       int err;
+       FILE_RENAME_INFO_VVS rename_info;
+       HANDLE f_handle;
+       wchar_t from_w[MAX_PATH];
+
+
+       if (MultiByteToWideChar(CP_ACP, 0, (LPCCH)from, -1, (LPWSTR)from_w, 
MAX_PATH) == 0) return -1;
+       if (MultiByteToWideChar(CP_ACP, 0, (LPCCH)to, -1, 
(LPWSTR)rename_info.FileName, MAX_PATH) == 0) 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;
+       }
+
+
+       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 = wcslen(rename_info.FileName);;
+       
+       if (!SetFileInformationByHandle(f_handle, FileRenameInfoEx, 
&rename_info, sizeof(FILE_RENAME_INFO_VVS)))
+       {
+               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 +124,16 @@ pgrename(const char *from, const char *to)
 {
        int                     loops = 0;
 
+       /*
+       * Calls pgrename_win10 on Windows 10 and later when _WIN32_WINNT >= 
_WIN32_WINNT_WIN10.
+       */
+#if defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 && 
!defined(__CYGWIN__)
+       if (IsWindows10OrGreater()) {
+               if (pgrename_win10(from, to) == 0) return 0;
+       }
+#endif
+
+
        /*
         * We need to loop because even though PostgreSQL uses flags that allow
         * rename while the file is open, other applications might have the file
diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index ebe6530ba58..31b67ee5b03 100644
--- a/src/tools/msvc/MSBuildProject.pm
+++ b/src/tools/msvc/MSBuildProject.pm
@@ -43,6 +43,7 @@ EOF
   </ItemGroup>
   <PropertyGroup Label="Globals">
     <ProjectGuid>$self->{guid}</ProjectGuid>
+    <GenerateManifest>false</GenerateManifest>
 EOF
        # Check whether WindowsSDKVersion env variable is present.
        # Add WindowsTargetPlatformVersion node if so.
diff --git a/src/tools/msvc/Project.pm b/src/tools/msvc/Project.pm
index 2f1679ab44f..de6201e89b0 100644
--- a/src/tools/msvc/Project.pm
+++ b/src/tools/msvc/Project.pm
@@ -352,6 +352,9 @@ sub AddResourceFile
                        }
                        print $o $_;
                }
+               # 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
index 00000000000..fd3a344bee0
--- /dev/null
+++ b/src/port/win10.manifest
@@ -0,0 +1,50 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1" 
xmlns:asmv3="urn:schemas-microsoft-com:asm.v3" manifestVersion="1.0">
+
+  <!-- Enable use of version 6 of the common controls (Win XP and later) -->
+  <dependency>
+    <dependentAssembly>
+      <assemblyIdentity type="win32"
+                        name="Microsoft.Windows.Common-Controls"
+                        version="6.0.0.0"
+                        processorArchitecture="*"
+                        publicKeyToken="6595b64144ccf1df"
+                        language="*" />
+    </dependentAssembly>
+  </dependency>
+
+  <!-- Indicate UAC compliance, with no need for elevated privileges (Win 
Vista and later) -->
+  <!-- (if you need enhanced privileges, set the level to "highestAvailable" 
or "requireAdministrator") -->
+  <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
+    <security>
+      <requestedPrivileges>
+        <requestedExecutionLevel level="asInvoker" uiAccess="false" />
+      </requestedPrivileges>
+    </security>
+  </trustInfo>
+
+  <!-- Indicate high API awareness (Win Vista and later) -->
+  <!-- (if you support per-monitor high DPI, set this to "True/PM") -->
+  <application xmlns="urn:schemas-microsoft-com:asm.v3">
+    <windowsSettings>
+      <dpiAware 
xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings";>false</dpiAware>
+    </windowsSettings>
+  </application>
+
+  <!-- Declare support for various versions of Windows -->
+  <ms_compatibility:compatibility 
xmlns:ms_compatibility="urn:schemas-microsoft-com:compatibility.v1" 
xmlns="urn:schemas-microsoft-com:compatibility.v1">
+    <ms_compatibility:application>
+      <!-- Windows Vista/Server 2008 -->
+      <ms_compatibility:supportedOS 
Id="{e2011457-1546-43c5-a5fe-008deee3d3f0}" />
+      <!-- Windows 7/Server 2008 R2 -->
+      <ms_compatibility:supportedOS 
Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}" />
+      <!-- Windows 8/Server 2012 -->
+      <ms_compatibility:supportedOS 
Id="{4a2f28e3-53b9-4441-ba9c-d69d4a4a6e38}" />
+      <!-- Windows 8.1/Server 2012 R2 -->
+      <ms_compatibility:supportedOS 
Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}" />
+      <!-- Windows 10 -->
+      <ms_compatibility:supportedOS 
Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}" />
+    </ms_compatibility:application>
+  </ms_compatibility:compatibility>
+
+</assembly>

Reply via email to