Re: pg_trgm comparison bug on cross-architecture replication due to different char implementation
Masahiko Sawada <sawada.mshk@gmail.com>
On Wed, May 1, 2024 at 2:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Alexander Korotkov <aekorotkov@gmail.com> writes: > > I agree that storing char signedness might seem weird. But it appears > > that we already store indexes that depend on char signedness. So, > > it's effectively property of bits-on-disk even though it affects > > indirectly. Then I see two options to make the picture consistent. > > 1) Assume that char signedness is somehow a property of bits-on-disk > > even though it's weird. Then pg_trgm indexes are correct, but we need > > to store char signedness in pg_control. > > 2) Assume that char signedness is not a property of bits-on-disk. > > Then pg_trgm indexes are buggy and need to be fixed. > > What do you think? > > The problem with option (2) is the assumption that pg_trgm's behavior > is the only bug of this kind, either now or in the future. I think > that's just about an impossible standard to meet, because there's no > realistic way to test whether char signedness is affecting things. > (Sure, you can compare results across platforms, but maybe you > just didn't test the right case.) > > Also, the bigger picture here is the seeming assumption that "if > we change pg_trgm then it will be safe to replicate from x86 to > arm". I don't believe that that's a good idea and I'm unwilling > to promise that it will work, regardless of what we do about > char signedness. That being the case, I don't want to invest a > lot of effort in the signedness issue. I think that the char signedness issue is an issue also for developers (and extension authors) since it could lead to confusion and potential bugs in the future due to that. x86 developers would think of char as always being signed and write code that will misbehave on arm machines. For example, since logical replication should behave correctly even in cross-arch replication all developers need to be aware of that. I thought of using the -funsigned-char (or -fsigned-char) compiler flag to avoid that but it would have a broader impact not only on indexes. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
Commits
-
pg_upgrade: Check for the expected error message in TAP tests.
- f52345995d36 18.0 landed
-
Fix a typo in 005_char_signedness.pl test.
- 945a9e3832c3 18.0 landed
-
Add test 005_char_signedness.pl to meson.build.
- 78d3f4889502 18.0 landed
-
Fix an issue with index scan using pg_trgm due to char signedness on different architectures.
- dfd8e6c73eea 18.0 landed
-
pg_upgrade: Add --set-char-signedness to set the default char signedness of new cluster.
- 1aab6805919b 18.0 landed
-
pg_upgrade: Preserve default char signedness value from old cluster.
- a8238f87f980 18.0 landed
-
pg_resetwal: Add --char-signedness option to change the default char signedness.
- 30666d1857d7 18.0 landed
-
Add default_char_signedness field to ControlFileData.
- 44fe30fdab67 18.0 landed
-
Remove unneeded nbtree array preprocessing assert.
- 480bc6e3ed3a 17.0 cited