Re: [PATCH v1] remove redundant check of item pointer

On Thu, Jul 14, 2022 at 3:31 PM Bruce Momjian <bruce@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