Simplify pg_collation.collversion for Windows libc

Started by Daniel Veriteover 2 years ago3 messages
#1Daniel Verite
daniel@manitou-mail.org
1 attachment(s)

Hi,

Currently the libc collation version for Windows has two components
coming from the NLSVERSIONINFOEX structure [1]https://learn.microsoft.com/en-us/windows/win32/api/winnls/ns-winnls-nlsversioninfoex
dwNLSVersion and dwDefinedVersion

So we get version numbers looking like this (with 16 beta1):

postgres=# select collversion,count(*) from pg_collation group by
collversion;
collversion | count
---------------+-------
| 5
1539.5,1539.5 | 1457
(2 rows)

According to [1]https://learn.microsoft.com/en-us/windows/win32/api/winnls/ns-winnls-nlsversioninfoex the second number is obsolete, and AFAICS we should
expose only the first.

<quote>
dwDefinedVersion

Defined version. This value is used to track changes in the repertoire
of Unicode code points. The value increments when the Unicode
repertoire is extended, for example, if more characters are defined.

Starting with Windows 8: Deprecated. Use dwNLSVersion instead.
</quote>

PFA a patch implementing that suggestion.

[1]: https://learn.microsoft.com/en-us/windows/win32/api/winnls/ns-winnls-nlsversioninfoex
https://learn.microsoft.com/en-us/windows/win32/api/winnls/ns-winnls-nlsversioninfoex

Best regards,
--
Daniel Vérité
https://postgresql.verite.pro/
Twitter: @DanielVerite

Attachments:

windows-collversion.patchtext/plainDownload
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 31e3b16ae0..3a6b00ab14 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1717,11 +1717,9 @@ get_collation_actual_version(char collprovider, const char *collcollate)
 							collcollate,
 							GetLastError())));
 		}
-		collversion = psprintf("%lu.%lu,%lu.%lu",
+		collversion = psprintf("%lu.%lu",
 							   (version.dwNLSVersion >> 8) & 0xFFFF,
-							   version.dwNLSVersion & 0xFF,
-							   (version.dwDefinedVersion >> 8) & 0xFFFF,
-							   version.dwDefinedVersion & 0xFF);
+							   version.dwNLSVersion & 0xFF);
 #endif
 	}
 
#2Thomas Munro
thomas.munro@gmail.com
In reply to: Daniel Verite (#1)
Re: Simplify pg_collation.collversion for Windows libc

On Mon, Jun 5, 2023 at 12:56 PM Daniel Verite <daniel@manitou-mail.org> wrote:

postgres=# select collversion,count(*) from pg_collation group by
collversion;
collversion | count
---------------+-------
| 5
1539.5,1539.5 | 1457
(2 rows)

According to [1] the second number is obsolete, and AFAICS we should
expose only the first.

Would it be a good idea to remove or ignore the trailing /,*$/
somewhere, perhaps during pg_upgrade, to avoid bogus version mismatch
warnings?

#3Peter Eisentraut
peter@eisentraut.org
In reply to: Thomas Munro (#2)
Re: Simplify pg_collation.collversion for Windows libc

On 06.06.23 03:21, Thomas Munro wrote:

On Mon, Jun 5, 2023 at 12:56 PM Daniel Verite <daniel@manitou-mail.org> wrote:

postgres=# select collversion,count(*) from pg_collation group by
collversion;
collversion | count
---------------+-------
| 5
1539.5,1539.5 | 1457
(2 rows)

According to [1] the second number is obsolete, and AFAICS we should
expose only the first.

Would it be a good idea to remove or ignore the trailing /,*$/
somewhere, perhaps during pg_upgrade, to avoid bogus version mismatch
warnings?

I wonder whether it's worth dealing with this, versus just leaving it
all alone.