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

From: Tom Lane <[email-masked]>
Date: Tue, 10 Sep 2024 02:25:35 -0400

Masahiko Sawada <[email-masked]> writes:
> On Mon, Sep 9, 2024 at 4:42 PM Tom Lane <[email-masked]> wrote:
>> Do you have an idea for how we'd get
>> this to happen during pg_upgrade, exactly?

> What I was thinking is that we have "pg_dump --binary-upgrade" emit a
> function call, say "SELECT binary_upgrade_update_gin_meta_page()" for
> each GIN index, and the meta pages are updated when restoring the
> schemas.

Hmm, but ...

1. IIRC we don't move the relation files into the new cluster until
after we've run the schema dump/restore step.  I think this'd have to
be driven in some other way than from the pg_dump output.  I guess we
could have pg_upgrade start up the new postmaster and call a function
in each DB, which would have to scan for GIN indexes by itself.

2. How will this interact with --link mode?  I don't see how it
doesn't involve scribbling on files that are shared with the old
cluster, leading to possible problems if the pg_upgrade fails later
and the user tries to go back to using the old cluster.  It's not so
much the metapage update that is worrisome --- we're assuming that
that will modify storage that's unused in old versions.  But the
change would be unrecorded in the old cluster's WAL, which sounds
risky.

Maybe we could get away with forcing --copy mode for affected
indexes, but that feels a bit messy.  We'd not want to do it
for unaffected indexes, so the copying code would need to know
a great deal about this problem.

			regards, tom lane

---

From: Masahiko Sawada <[email-masked]>
Date: Tue, 10 Sep 2024 11:31:46 -0700

> in each DB, which would have to scan for GIN indexes by itself.

You're right.

> a great deal about this problem.

Good point. I agree that it would not be a very good idea to force --copy mode.

An alternative way would be that we store the char signedness in the
control file, and gin_trgm_ops opclass reads it if the bytes in the
meta page shows 'unset'. The char signedness in the control file
doesn't mean to be used for the compatibility check for physical
replication but used as a hint. But it also could be a bit messy,
though.

Regards,

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

---

From: Tom Lane <[email-masked]>
Date: Tue, 10 Sep 2024 14:57:55 -0400

> replication but used as a hint. But it also could be a bit messy,

Yeah, that seems like it could work.  But are we sure that replicas
get a copy of the primary's control file rather than creating their
own?

			regards, tom lane

---

From: Masahiko Sawada <[email-masked]>
Date: Tue, 10 Sep 2024 14:51:51 -0700

> get a copy of the primary's control file rather than creating their

Yes, I think so. Since at least the system identifiers of primary and
replicas must be identical for physical replication, if replicas use
their own control files then they cannot start the replication.

Regards,

---

From: Tom Lane <[email-masked]>
Date: Tue, 10 Sep 2024 17:56:47 -0400

> their own control files then they cannot start the replication.

Got it.  So now I'm wondering if we need all the complexity of storing
stuff in the GIN metapages.  Could we simply read the (primary's)
signedness out of pg_control and use that?  We might need some caching
mechanism to make that cheap enough, but accessing the current index's
metapage is far from free either.

			regards, tom lane