using SysCacheGetAttrNotNull in place of heap_getattr

I was about to push a quick patch to replace the use of heap_getattr()
in get_primary_key_attnos() with SysCacheGetAttrNotNull(), because that
makes the code a few lines shorter and AFAICS there's no downside.
However, I realized that the commit that added the function
(d435f15fff3c) did not make any such change at all -- it only changed
SysCacheGetAttr calls to use the new function, but no heap_getattr.
And we don't seem to have added such calls after.

Essentially the possibly contentious point is that the tuple we'd be
deforming did not come from syscache, but from a systable scan, so
calling a syscache function on it could be seen as breaking some API.
(Of course, this only works if there is a syscache on the given
relation.)

But we do have precedent: for example RelationGetFKeyList uses a sysscan
to feed DeconstructFkConstraintRow(), which extracts several attributes
that way using the CONSTROID syscache.

Does anybody think this could be a problem, if we extended it to be more
widely used?

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Los cuentos de hadas no dan al niño su primera idea sobre los monstruos.
Lo que le dan es su primera idea de la posible derrota del monstruo."
(G. K. Chesterton)