From f354092916a66b64310676fcba6a399bc8a29b35 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Wed, 25 Sep 2024 11:08:57 -0700 Subject: [PATCH v3 1/5] Add default_char_signedness field to ControlFileData. The signedness of the 'char' type in C is implementation-dependent. For instance, 'signed char' is used by default on x86 CPUs, while 'unsigned char' is used on aarch CPUs. Previously, we accidentally let C implementation signedness affect persistent data. This led to inconsistent results when comparing char data across different platforms. This commit introduces a new 'default_char_signedness' field in ControlFileData to store the signedness of the 'char' type. While this change does not encourage the use of 'char' without explicitly specifying its signedness, this field can be used as a hint to ensure consistent behavior for pre-v18 data files that store data sorted by the 'char' type on disk (e.g., GIN and GiST indexes), especially in cross-platform replication scenarios. Newly created database clusters unconditionally set the default char signedness to true. pg_upgrade (with an upcoming commit) changes this flag for clusters if the source database cluster has signedness=false. As a result, signedness=false setting will become rare over time. If we had known about the problem during the last development cycle that forced initdb (v8.3), we would have made all clusters signed or all clusters unsigned. Making pg_upgrade the only source of signedness=false will cause the population of database clusters to converge toward that retrospective ideal. XXX bump catversion. Reviewed-by: Noah Misch Discussion: https://postgr.es/m/CB11ADBC-0C3F-4FE0-A678-666EE80CBB07%40amazon.com --- doc/src/sgml/func.sgml | 5 ++++ src/backend/access/transam/xlog.c | 40 +++++++++++++++++++++++++ src/backend/utils/misc/pg_controldata.c | 7 +++-- src/bin/pg_controldata/pg_controldata.c | 2 ++ src/include/access/xlog.h | 1 + src/include/catalog/pg_control.h | 6 ++++ src/include/catalog/pg_proc.dat | 6 ++-- 7 files changed, 62 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 47370e581ae..b991d663cb5 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -27935,6 +27935,11 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres} integer + + default_char_signedness + bool + + diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index bf3dbda901d..83122a4d68b 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -4263,6 +4263,33 @@ WriteControlFile(void) ControlFile->float8ByVal = FLOAT8PASSBYVAL; + /* + * Initialize the default 'char' signedness. + * + * The signedness of the char type is implementation-defined. For instance + * on x86 architecture CPUs, the char data type is typically treated as + * signed by default, whereas on aarch architecture CPUs, it is typically + * treated as unsigned by default. In v17 or earlier, we accidentally let + * C implementation signedness affect persistent data. This led to + * inconsistent results when comparing char data across different + * platforms. + * + * This flag can be used as a hint to ensure consistent behavior for + * pre-v18 data files that store data sorted by the 'char' type on disk, + * especially in cross-platform replication scenarios. + * + * Newly created database clusters unconditionally set the default char + * signedness to true. pg_upgrade changes this flag for clusters that were + * initialized on signedness=false platforms. As a result, + * signedness=false setting will become rare over time. If we had known + * about this problem during the last development cycle that forced initdb + * (v8.3), we would have made all clusters signed or all clusters + * unsigned. Making pg_upgrade the only source of signedness=false will + * cause the population of database clusters to converge toward that + * retrospective ideal. + */ + ControlFile->default_char_signedness = true; + /* Contents are protected with a CRC */ INIT_CRC32C(ControlFile->crc); COMP_CRC32C(ControlFile->crc, @@ -4591,6 +4618,19 @@ DataChecksumsEnabled(void) return (ControlFile->data_checksum_version > 0); } +/* + * Return true if the cluster was initialized on a platform where the + * default signedness of char is "signed". This function exists for code + * that deals with pre-v18 data files that store data sorted by the 'char' + * type on disk (e.g., GIN and GiST indexes). See the comments in + * WriteControlFile() for details. + */ +bool +GetDefaultCharSignedness(void) +{ + return ControlFile->default_char_signedness; +} + /* * Returns a fake LSN for unlogged relations. * diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c index 9dfba499c13..6d036e3bf32 100644 --- a/src/backend/utils/misc/pg_controldata.c +++ b/src/backend/utils/misc/pg_controldata.c @@ -203,8 +203,8 @@ pg_control_recovery(PG_FUNCTION_ARGS) Datum pg_control_init(PG_FUNCTION_ARGS) { - Datum values[11]; - bool nulls[11]; + Datum values[12]; + bool nulls[12]; TupleDesc tupdesc; HeapTuple htup; ControlFileData *ControlFile; @@ -254,6 +254,9 @@ pg_control_init(PG_FUNCTION_ARGS) values[10] = Int32GetDatum(ControlFile->data_checksum_version); nulls[10] = false; + values[11] = BoolGetDatum(ControlFile->default_char_signedness); + nulls[11] = false; + htup = heap_form_tuple(tupdesc, values, nulls); PG_RETURN_DATUM(HeapTupleGetDatum(htup)); diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c index 93a05d80ca7..ac74c2c5e9d 100644 --- a/src/bin/pg_controldata/pg_controldata.c +++ b/src/bin/pg_controldata/pg_controldata.c @@ -325,6 +325,8 @@ main(int argc, char *argv[]) (ControlFile->float8ByVal ? _("by value") : _("by reference"))); printf(_("Data page checksum version: %u\n"), ControlFile->data_checksum_version); + printf(_("Default char data signedness: %s\n"), + (ControlFile->default_char_signedness ? _("signed") : _("unsigned"))); printf(_("Mock authentication nonce: %s\n"), mock_auth_nonce_str); return 0; diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 4411c1468ac..d313099c027 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -231,6 +231,7 @@ extern XLogRecPtr GetXLogWriteRecPtr(void); extern uint64 GetSystemIdentifier(void); extern char *GetMockAuthenticationNonce(void); extern bool DataChecksumsEnabled(void); +extern bool GetDefaultCharSignedness(void); extern XLogRecPtr GetFakeLSNForUnloggedRel(void); extern Size XLOGShmemSize(void); extern void XLOGShmemInit(void); diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h index 3797f25b306..0cb86faa43b 100644 --- a/src/include/catalog/pg_control.h +++ b/src/include/catalog/pg_control.h @@ -221,6 +221,12 @@ typedef struct ControlFileData /* Are data pages protected by checksums? Zero if no checksum version */ uint32 data_checksum_version; + /* + * True if the default signedness of char is "signed" on a platform where + * the cluster is initialized. + */ + bool default_char_signedness; + /* * Random nonce, used in authentication requests that need to proceed * based on values that are cluster-unique, like a SASL exchange that diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index ba02ba53b29..473e9defefc 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -12175,9 +12175,9 @@ descr => 'pg_controldata init state information as a function', proname => 'pg_control_init', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{int4,int4,int4,int4,int4,int4,int4,int4,int4,bool,int4}', - proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{max_data_alignment,database_block_size,blocks_per_segment,wal_block_size,bytes_per_wal_segment,max_identifier_length,max_index_columns,max_toast_chunk_size,large_object_chunk_size,float8_pass_by_value,data_page_checksum_version}', + proallargtypes => '{int4,int4,int4,int4,int4,int4,int4,int4,int4,bool,int4,bool}', + proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{max_data_alignment,database_block_size,blocks_per_segment,wal_block_size,bytes_per_wal_segment,max_identifier_length,max_index_columns,max_toast_chunk_size,large_object_chunk_size,float8_pass_by_value,data_page_checksum_version,default_char_signedness}', prosrc => 'pg_control_init' }, # subscripting support for built-in types -- 2.43.5