From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Refactoring SysCacheGetAttr to know when attr cannot be NULL |
Date: | 2023-03-13 13:19:07 |
Message-ID: | 6F44C9AE-50C1-4062-B8E6-0916F63791DD@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 2 Mar 2023, at 15:44, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> 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. That's easily
> convertible to a name, and it doesn't tend to move around across PG
> versions, neither of which are true for syscache IDs.
>
> Also, I'm fairly unconvinced that it's a "can't happen" --- this
> would be very likely to fire as a result of catalog corruption,
> so it would be good if it's at least minimally interpretable
> by a non-expert. Given that we'll now have just one copy of the
> code, ISTM there's a good case for doing the small extra work
> to report catalog and column by name.
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.
--
Daniel Gustafsson
Attachment | Content-Type | Size |
---|---|---|
v3-0001-Add-SysCacheGetAttrNotNull-for-guaranteed-not-nul.patch | application/octet-stream | 69.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jonathan S. Katz | 2023-03-13 13:26:45 | Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences |
Previous Message | Amit Kapila | 2023-03-13 13:11:30 | Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher |