From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] Check more invariants during syscache initialization |
Date: | 2023-07-25 23:45:10 |
Message-ID: | ZMBehh8vkUtkr3dq@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jul 25, 2023 at 02:35:31PM +0300, Aleksander Alekseev wrote:
>> - Assert(cacheinfo[cacheId].reloid != 0);
>> + Assert(cacheinfo[cacheId].reloid != InvalidOid);
>> + Assert(cacheinfo[cacheId].indoid != InvalidOid);
>> No objections about checking for the index OID given out to catch
>> any failures at an early stage before doing an actual lookup? I guess
>> that you've added an incorrect entry and noticed the problem only when
>> triggering a syscache lookup for the new incorrect entry?
>> + Assert(key[i] != InvalidAttrNumber);
>>
>> Yeah, same here.
>
> Not sure if I understand your question or suggestion. Thes proposed
> patch adds Asserts() to InitCatalogCache() and InitCatCache() called
> by it, before any actual lookups.
That was more a question. I was wondering if it was something you've
noticed while working on a different patch because you somewhat
assigned incorrect values in the syscache array, but it looks like you
have noticed that while scanning the code.
>> + Assert((cacheinfo[cacheId].nkeys > 0) && (cacheinfo[cacheId].nkeys <= 4));
>>
>> Is this addition actually useful?
>
> I believe it is. One can mistakenly use 5+ keys in cacheinfo[] declaration, e.g:
Still it's hard to miss at compile time. I think that I would remove
this one.
> All in all I'm not claiming that these proposed new Asserts() make a
> huge difference. I merely noticed that InitCatalogCache checks only
> cacheinfo[cacheId].reloid. Checking the rest of the values would be
> helpful for the developers and will not impact the performance of the
> release builds.
No counter-arguments on that. The checks for the index OID and the
keys allow to catch failures in these structures at an earlier stage
when initializing a backend. Agreed that it can be useful for
developers.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2023-07-25 23:47:41 | Re: POC, WIP: OR-clause support for indexes |
Previous Message | Nathan Bossart | 2023-07-25 23:24:26 | Re: [PATCH] Add function to_oct |