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

On Tue, Oct 01, 2024 at 11:55:48AM -0700, Masahiko Sawada wrote:
> On Mon, Sep 30, 2024 at 11:49 AM Noah Misch <noah@leadboat.com> wrote:
> > This has initdb set the field to the current C implementation's signedness.  I
> > wonder if, instead, initdb should set signedness=true unconditionally.  Then
> > the only source of signedness=false will be pg_upgrade.
> >
> > Advantage: signedness=false will get rarer 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.
> 
> I think it's a good idea. Being able to treat one case as a rarer one
> rather than treating both cases equally may have various advantages in
> the future, for example when developing or investigating a problem.
> 
> > Disadvantage: testing signedness=false will require more planning.
> 
> If testing signedness=false always requires pg_upgrade, there might be
> some cumbersomeness. Inventing a testing-purpose-only tool (e.g., a
> CLI program) that changes the signedness might make tests easier.
> 
> > What other merits should we consider as part of deciding?
> 
> Considering that the population of database cluster signedness will
> converge to signedness=true in the future, we can consider using
> -fsigned-char to prevent similar problems for the future. We need to
> think about possible side-effects as well, though.

It's good to think about -fsigned-char.  While I find it tempting, several
things would need to hold for us to benefit from it:

- Every supported compiler has to offer it or an equivalent.
- The non-compiler parts of every supported C implementation need to
  cooperate.  For example, CHAR_MIN must change in response to the flag.  See
  the first comment in cash_in().
- Libraries we depend on can't do anything incompatible with it.

Given that, I would lean toward not using -fsigned-char.  It's unlikely all
three things will hold.  Even if they do, the benefit is not large.