On Fri, 24 Mar 2023 at 20:31, Peter Eisentraut <peter.eisentr...@enterprisedb.com> wrote: > > On 23.03.23 09:52, David Rowley wrote: > > One thing I thought about while looking is it stage 2 might do > > something similar for SearchSysCacheN. I then wondered if we're more > > likely to want to keep the localised __FILE__, __LINE__ and __func__ > > in the elog for those or not. It's probably less important that we're > > losing those for this change, but worth noting here at least in case > > nobody else thought of it. > > I don't follow what you are asking for here.
I had two points: 1. Doing something similar for SearchSysCache1 and co might be a good phase two to this change. 2. With the change Daniel is proposing here, \set VERBOSITY verbose is not going to print as useful information to tracking down where any unexpected nulls in the catalogue originates. For #2, I don't think that's necessarily a problem. I can think of two reasons why SysCacheGetAttrNotNull might throw an ERROR: a) We used SysCacheGetAttrNotNull() when we should have used SysCacheGetAttr(). b) Catalogue corruption. A more localised ERROR message might just help more easily tracking down type a) problems. I imagine it won't be too difficult to just grep for all the SysCacheGetAttrNotNull calls for the particular nullable column to find the one causing the issue. For b), the error message in SysCacheGetAttrNotNull is sufficient without needing to know where the SysCacheGetAttrNotNull call came from. David