Re: pg_trgm comparison bug on cross-architecture replication due to different char implementation

On Tue, Feb 18, 2025 at 3:23 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
>
> Apart from two doc issues, this is ready:
>
> On Tue, Feb 18, 2025 at 01:23:20PM -0800, Masahiko Sawada wrote:
> > On Mon, Feb 17, 2025 at 2:57 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > > On Fri, Jan 17, 2025 at 05:11:41PM -0800, Masahiko Sawada wrote:
>
> > + However, when upgrading from <productname>PostgreSQL</productname> 17 or,
> > + earlier <application>pg_upgrade</application> adopts the char signedness
>
> s/or, earlier/or earlier,/
>
> > --- a/doc/src/sgml/ref/pg_resetwal.sgml
> > +++ b/doc/src/sgml/ref/pg_resetwal.sgml
> > @@ -171,6 +171,22 @@ PostgreSQL documentation
> > </para>
> >
> > <variablelist>
> > + <varlistentry>
> > + <term><option>--char-signedness=<replaceable class="parameter">option</replaceable></option></term>
> > + <listitem>
> > + <para>
> > + Manually set the default char signedness. Possible values are
> > + <literal>signed</literal> and <literal>unsigned</literal>.
> > + </para>
> > + <para>
> > + A safe value for this option is, if known, the default char signedness
> > + of the platform where the database cluster was initialized. However,
>
> Only if initialized on v17 or earlier. I recommend this edit:
>
> diff --git a/doc/src/sgml/ref/pg_resetwal.sgml b/doc/src/sgml/ref/pg_resetwal.sgml
> index a72678d..dd011d2 100644
> --- a/doc/src/sgml/ref/pg_resetwal.sgml
> +++ b/doc/src/sgml/ref/pg_resetwal.sgml
> @@ -179,8 +179,11 @@ PostgreSQL documentation
> <literal>signed</literal> and <literal>unsigned</literal>.
> </para>
> <para>
> - A safe value for this option is, if known, the default char signedness
> - of the platform where the database cluster was initialized. However,
> + For a database cluster that <command>pg_upgrade</command> upgraded from
> + a <productname>PostgreSQL</productname> version before 18, the safe
> + value would be the default <type>char</type> signedness of the platform
> + that ran the cluster before that upgrade. For all other
> + clusters, <literal>signed</literal> would be the safe value. However,
> this option is exclusively for use with <command>pg_upgrade</command>
> and should not normally be used manually.
> </para>

Thank you for reviewing the patches. I've fixed these issues and
attached the updated patches.

I have one question about the 0001 patch; since we add
'default_char_signedness' field to ControlFileData do we need to bump
PG_CONTROL_VERSION? We have comments about bumping PG_CONTROL_VERSION
when changing CheckPoint struct or DBState enum so it seems likely but
I'd like to confirm just in case that we need to bump
PG_CONTROL_VERSION also when changing ControlFileData. If we need, can
we bump it to 1800? or 1701?

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com