On Thu, Jul 14, 2022 at 3:31 PM Bruce Momjian <br...@momjian.us> wrote: > On Wed, Apr 27, 2022 at 08:04:00PM +0800, Junwang Zhao wrote: > for non-Assert builds, ItemPointerGetOffsetNumberNoCheck() and > ItemPointerGetOffsetNumber() are the same, so I don't see the point to > making this change. Frankly, I don't know why we even have two > functions for this. I am guessing ItemPointerGetOffsetNumberNoCheck is > for cases where you have an Assert build and do not want the check.
Sometimes we use ItemPointerData for things that aren't actually TIDs. For example, both GIN and B-Tree type-pun the ItemPointerData field from the Indextuple struct. Plus we do something like that with UPDATEs that affect a partitioning key in a partitioned table. The proposal doesn't seem like an improvement. Technically the assertion cannot possibly fail here because the earlier assertion would always fail instead, so strictly speaking it is redundant -- at least right now. That is true. But it seems much more important to be consistent about which variant to use. Especially because there is obviously no overhead in builds without assertions enabled. -- Peter Geoghegan
