On Tue, 23 Apr 2024 at 12:37, Heikki Linnakangas <hlinn...@iki.fi> wrote:
> This is really a bug fix. It didn't matter when TransactionId and > MultiXactOffset were both typedefs of uint32, but it was always wrong. > The argument name 'xid' is also misleading. > > I think there are some more like that, MXOffsetToFlagsBitShift for example. Yeah, I always thought so too. I believe, this is just a copy-paste. You mean, it is worth creating a separate CF entry for these fixes? On Tue, 23 Apr 2024 at 16:03, Andrey M. Borodin <x4...@yandex-team.ru> wrote: > BTW as a side note... I see lot's of casts to (unsigned long long), can't > we just cast to MultiXactOffset? > Actually, first versions of the 64xid patch set have such a cast to types TransactionID, MultiXact and so on. But, after some discussions, we are switched to unsigned long long cast. Unfortunately, I could not find an exact link for that discussion. On the other hand, such a casting is already used throughout the code. So, just for the sake of the consistency, I would like to stay with these casts. On Tue, 23 Apr 2024 at 16:03, wenhui qiu <qiuwenhu...@gmail.com> wrote: > Hi Maxim Orlov > Thank you so much for your tireless work on this. Increasing the WAL > size by a few bytes should have very little impact with today's disk > performance(Logical replication of this feature wal log is also increased a > lot, logical replication is a milestone new feature, and the community has > been improving the logical replication of functions),I believe removing > troubled postgresql Transaction ID Wraparound was also a milestone new > feature adding a few bytes is worth it! > I'm 100% agree. Maybe, I should return to this approach and find some benefits for having FXIDs in WAL.