Re: Refactoring SysCacheGetAttr to know when attr cannot be NULL

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Refactoring SysCacheGetAttr to know when attr cannot be NULL
Date: 2023-02-28 23:20:21
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
> The attached refactoring introduce SysCacheGetAttrNotNull as a wrapper around
> SysCacheGetAttr where a NULL value triggers an elog().

+1, seems like a good idea. (I didn't review the patch details.)

> This will reduce granularity of error messages, and as the patch sits now it
> does so a lot since the message is left to work on - I wanted to see if this
> was at all seen as a net positive before spending time on that part. I chose
> an elog since I as a user would prefer to hit an elog instead of a silent keep
> going with an assert, this is of course debateable.

I'd venture that the Assert cases are mostly from laziness, and
that once we centralize this it's plenty worthwhile to generate
a decent elog message. You ought to be able to look up the
table and column name from the info that is at hand.

Also ... at least in assert-enabled builds, maybe we could check that
the column being fetched this way is actually marked attnotnull?
That would help to catch misuse.

regards, tom lane

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2023-02-28 23:38:21 Re: [PoC] Let libpq reject unexpected authentication requests
Previous Message Daniel Gustafsson 2023-02-28 23:12:11 Re: Marking options deprecated in help output