Justin Pryzby writes:
> On Mon, Mar 13, 2023 at 02:19:07PM +0100, Daniel Gustafsson wrote:
>> + elog(ERROR,
>> +"unexpected NULL value in cached tuple for
>> pg_catalog.%s.%s",
>> +get_rel_name(cacheinfo[cacheId].reloid),
> Question:
On Mon, Mar 13, 2023 at 02:19:07PM +0100, Daniel Gustafsson wrote:
> > On 2 Mar 2023, at 15:44, Tom Lane wrote:
> >
> > Peter Eisentraut writes:
> >> I think an error message like
> >> "unexpected null value in system cache %d column %d"
> >> is sufficient. Since these are "can't happen" er
> On 14 Mar 2023, at 08:00, Peter Eisentraut
> wrote:
> I prefer to use "null value" for SQL null values, and NULL for the C symbol.
Thats a fair point, I agree with that.
> I'm a bit hesitant about hardcoding pg_catalog here. That happens to be
> true, of course, but isn't actually enforced
> On 24 Mar 2023, at 21:59, David Rowley wrote:
> On Fri, 24 Mar 2023 at 20:31, Peter Eisentraut
> 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
>>> li
On Fri, 24 Mar 2023 at 20:31, Peter Eisentraut
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__
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 import
On Tue, 14 Mar 2023 at 02:19, Daniel Gustafsson wrote:
> Rebased v3 on top of recent conflicting ICU changes causing the patch to not
> apply anymore. Also took another look around the tree to see if there were
> missed callsites but found none new.
I had a look at this. It generally seems like
On 13.03.23 14:19, Daniel Gustafsson wrote:
On 2 Mar 2023, at 15:44, Tom Lane wrote:
Peter Eisentraut writes:
I think an error message like
"unexpected null value in system cache %d column %d"
is sufficient. Since these are "can't happen" errors, we don't need to
spend too much extra ef
> On 2 Mar 2023, at 15:44, Tom Lane wrote:
>
> Peter Eisentraut writes:
>> I think an error message like
>> "unexpected null value in system cache %d column %d"
>> is sufficient. Since these are "can't happen" errors, we don't need to
>> spend too much extra effort to make it prettier.
>
Peter Eisentraut writes:
> I think an error message like
> "unexpected null value in system cache %d column %d"
> is sufficient. Since these are "can't happen" errors, we don't need to
> spend too much extra effort to make it prettier.
I'd at least like to see it give the catalog's OID. T
Daniel Gustafsson writes:
>> On 1 Mar 2023, at 00:20, Tom Lane wrote:
>> Also ... at least in assert-enabled builds, maybe we could check that
>> the column being fetched this way is actually marked attnotnull?
> We could, but that would limit the API to attnotnull columns, rather than when
> th
> On 2 Mar 2023, at 10:59, Peter Eisentraut
> wrote:
>
> On 28.02.23 21:14, Daniel Gustafsson wrote:
>> The attached refactoring introduce SysCacheGetAttrNotNull as a wrapper around
>> SysCacheGetAttr where a NULL value triggers an elog(). This removes a lot of
>> boilerplate error handling whi
> On 1 Mar 2023, at 00:20, Tom Lane wrote:
> 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.
We could, but that would limit the API to attnotnull columns, rather than when
On 28.02.23 21:14, Daniel Gustafsson wrote:
The attached refactoring introduce SysCacheGetAttrNotNull as a wrapper around
SysCacheGetAttr where a NULL value triggers an elog(). This removes a lot of
boilerplate error handling which IMO leads to increased readability as the
error handling *in the
> On 1 Mar 2023, at 21:04, Tom Lane wrote:
>
> Peter Eisentraut writes:
>> Yes please!
>
>> I have occasionally wondered whether just passing the isnull argument as
>> NULL would be sufficient, so we don't need a new function.
>
> I thought about that too. I think I prefer Daniel's formulati
Peter Eisentraut writes:
> Yes please!
> I have occasionally wondered whether just passing the isnull argument as
> NULL would be sufficient, so we don't need a new function.
I thought about that too. I think I prefer Daniel's formulation
with the new function, but I'm not especially set on th
On 28.02.23 21:14, Daniel Gustafsson wrote:
Today we have two fairly common patterns around extracting an attr from a
cached tuple:
a = SysCacheGetAttr(OID, tuple, Anum_pg_foo_bar, &isnull);
Assert(!isnull);
a = SysCacheGetAttr(OID, tuple, Anum_pg_foo_bar, &isnull);
if (isnull)
Daniel Gustafsson 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
Today we have two fairly common patterns around extracting an attr from a
cached tuple:
a = SysCacheGetAttr(OID, tuple, Anum_pg_foo_bar, &isnull);
Assert(!isnull);
a = SysCacheGetAttr(OID, tuple, Anum_pg_foo_bar, &isnull);
if (isnull)
elog(ERROR, "..");
The error message in the elog(
19 matches
Mail list logo
