On Wed, Jul 26, 2023 at 07:01:11PM +0300, Aleksander Alekseev wrote:
> Hi,
>
> > Shouldn't these be calling `OidIsValid(…)`, not comparing directly to
> > `InvalidOid`?
>
> They should, thanks. Here is the updated patch. I made sure there are
> no others != InvalidOid checks in syscache.c and
Hi,
> On Jul 27, 2023, at 09:05, Michael Paquier wrote:
>
> One reason is that this increases the odds of
> conflicts when backpatching on a stable branch.
Agree. We could suggest to use OidIsValid() for new patches during review.
Zhang Mingli
https://www.hashdata.xyz
On Wed, Jul 26, 2023 at 03:28:55PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Shouldn't these be calling `OidIsValid(…)`, not comparing directly to
> `InvalidOid`?
I think that they should use OidIsValid() on correctness ground, and
that's the style I prefer. Now, I don't think that these are
Hi,
> Shouldn't these be calling `OidIsValid(…)`, not comparing directly to
> `InvalidOid`?
They should, thanks. Here is the updated patch. I made sure there are
no others != InvalidOid checks in syscache.c and catcache.c which are
affected by my patch. I didn't change any other files since
Aleksander Alekseev writes:
> Hi Zhang,
>
>> That remind me to have a look other codes, and a grep search `oid != 0` show
>> there are several files using old != 0.
>>
>> ```
>> .//src/bin/pg_resetwal/pg_resetwal.c: if (set_oid != 0)
>> .//src/bin/pg_resetwal/pg_resetwal.c: if (set_oid != 0)
>>
Hi Zhang,
> That remind me to have a look other codes, and a grep search `oid != 0` show
> there are several files using old != 0.
>
> ```
> .//src/bin/pg_resetwal/pg_resetwal.c: if (set_oid != 0)
> .//src/bin/pg_resetwal/pg_resetwal.c: if (set_oid != 0)
> .//src/bin/pg_dump/pg_backup_tar.c: if
HI,
> On Jul 26, 2023, at 20:50, Aleksander Alekseev
> wrote:
>
> Hi Michael,
>
>> 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
Hi Michael,
> 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.
Oh, got it. That's correct.
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
Hi Michael,
> - 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
On Mon, Jul 24, 2023 at 09:58:15PM +0300, Aleksander Alekseev wrote:
> Currently InitCatalogCache() has only one Assert() for cacheinfo[]
> that checks .reloid. The proposed patch adds sanity checks for the
> rest of the fields.
- Assert(cacheinfo[cacheId].reloid != 0);
+
Hi,
Currently InitCatalogCache() has only one Assert() for cacheinfo[]
that checks .reloid. The proposed patch adds sanity checks for the
rest of the fields.
--
Best regards,
Aleksander Alekseev
v1-0001-Check-more-invariants-during-syscache-initializat.patch
Description: Binary data
12 matches
Mail list logo
