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@leadboat.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@leadboat.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