Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Em sex., 2 de fev. de 2024 às 03:48, Michael Paquier escreveu: > On Fri, Feb 02, 2024 at 12:04:39AM +0530, vignesh C wrote: > > The patch which you submitted has been awaiting your attention for > > quite some time now. As such, we have moved it to "Returned with > > Feedback" and removed it fro

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On Fri, Feb 02, 2024 at 12:04:39AM +0530, vignesh C wrote: > The patch which you submitted has been awaiting your attention for > quite some time now. As such, we have moved it to "Returned with > Feedback" and removed it from the reviewing queue. Depending on > timing, this may be reversible. Ki

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On Mon, 4 Sept 2023 at 21:40, Peter Eisentraut wrote: > > On 30.06.23 20:48, Karina Litskevich wrote: > > So as for me calling LockRelationForExtension() and > > UnlockRelationForExtension() > > without testing eb.rel first looks more like a bug here. However, they > > are never > > actually calle

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On 30.06.23 20:48, Karina Litskevich wrote: So as for me calling LockRelationForExtension() and UnlockRelationForExtension() without testing eb.rel first looks more like a bug here. However, they are never actually called with eb.rel=NULL because of the EB_* flags, so there is no bug here. I be

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Em qui., 6 de jul. de 2023 às 16:06, Gurjeet Singh escreveu: > On Thu, Jul 6, 2023 at 8:01 AM Karina Litskevich > wrote: > > > > > >> EB_SMGR and EB_REL are macros for making new structs. > >> IMO these are buggy, once make new structs without initializing all > fields. > >> Attached a patch to

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On Thu, Jul 6, 2023 at 8:01 AM Karina Litskevich wrote: > > >> EB_SMGR and EB_REL are macros for making new structs. >> IMO these are buggy, once make new structs without initializing all fields. >> Attached a patch to fix this and make more clear when rel or smgr is NULL. > > > As long as a struc

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Em sex., 30 de jun. de 2023 às 15:48, Karina Litskevich < litskevichkar...@gmail.com> escreveu: > Hi, > > Good catch. Indeed, eb.rel shouldn't be NULL there and the tests should be > unnecessary. > Thanks for the confirmation. > However, it doesn't follow from the code of these functions. > From

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Hi, Good catch. Indeed, eb.rel shouldn't be NULL there and the tests should be unnecessary. However, it doesn't follow from the code of these functions. >From what I can see eb.rel can be NULL in both of these functions. There is the following Assert in the beginning of the ExtendBufferedRelTo() f

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Em qua., 14 de jun. de 2023 às 13:32, Gurjeet Singh escreveu: > On Wed, Jun 14, 2023 at 5:12 AM Ranier Vilela wrote: > > > > Em qua., 14 de jun. de 2023 às 06:51, Richard Guo < > guofengli...@gmail.com> escreveu: > >> > >> > >> On Tue, Jun 13, 2023 at 3:39 PM Kyotaro Horiguchi < > horikyota@

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On Wed, Jun 14, 2023 at 5:12 AM Ranier Vilela wrote: > > Em qua., 14 de jun. de 2023 às 06:51, Richard Guo > escreveu: >> >> >> On Tue, Jun 13, 2023 at 3:39 PM Kyotaro Horiguchi >> wrote: >>> >>> Gurjeet has mentioned that eb.rel cannot be modified by another >>> process since the value or mem

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Em qua., 14 de jun. de 2023 às 06:51, Richard Guo escreveu: > > On Tue, Jun 13, 2023 at 3:39 PM Kyotaro Horiguchi > wrote: > >> Gurjeet has mentioned that eb.rel cannot be modified by another >> process since the value or memory is in the local stack, and I believe >> he's correct. >> >> If the

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On Tue, Jun 13, 2023 at 3:39 PM Kyotaro Horiguchi wrote: > Gurjeet has mentioned that eb.rel cannot be modified by another > process since the value or memory is in the local stack, and I believe > he's correct. > > If the pointed Relation had been blown out, eb.rel would be left > dangling, not

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

At Wed, 14 Jun 2023 10:01:59 +0900 (JST), Kyotaro Horiguchi wrote in > If we decide to remove it, the preceding blank line seems to be a > separator from the previous function call. So, we might want to Mmm. that is a bit short. Anyway I meant that the blank will become useless after removing t

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

At Tue, 13 Jun 2023 08:21:20 -0300, Ranier Vilela wrote in > Em ter., 13 de jun. de 2023 às 02:51, Gurjeet Singh > escreveu: > > > Please see attached v2 of the patch; it includes both occurrences of > > the spurious checks identified in this thread. > > > +1 > LGTM. > LockRela

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Em ter., 13 de jun. de 2023 às 02:51, Gurjeet Singh escreveu: > Please see attached v2 of the patch; it includes both occurrences of > the spurious checks identified in this thread. > +1 LGTM. regards, Ranier Vilela

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

At Tue, 13 Jun 2023 15:11:26 +0900, Michael Paquier wrote in > On Mon, Jun 12, 2023 at 10:51:24PM -0700, Gurjeet Singh wrote: > > To me, it looks like these checks are a result of code being > > copy-pasted from somewhere else, where this check might have been > > necessary. The checks are sure

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On Mon, Jun 12, 2023 at 10:51:24PM -0700, Gurjeet Singh wrote: > To me, it looks like these checks are a result of code being > copy-pasted from somewhere else, where this check might have been > necessary. The checks are sure not necessary at these spots. I am not completely sure based on my read

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On Mon, Jun 5, 2023 at 4:24 AM Ranier Vilela wrote: > Em seg., 5 de jun. de 2023 às 08:06, Ranier Vilela > escreveu: >> Em dom., 4 de jun. de 2023 às 23:37, Richard Guo >> escreveu: >>> On Sun, Jun 4, 2023 at 8:42 PM Ranier Vilela wrote: Hi, Per Coverity. At function

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Em seg., 5 de jun. de 2023 às 08:06, Ranier Vilela escreveu: > Em dom., 4 de jun. de 2023 às 23:37, Richard Guo > escreveu: > >> >> On Sun, Jun 4, 2023 at 8:42 PM Ranier Vilela wrote: >> >>> Hi, >>> >>> Per Coverity. >>> >>> At function ExtendBufferedRelShared, has a always true test. >>> eb.re

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Em dom., 4 de jun. de 2023 às 23:37, Richard Guo escreveu: > > On Sun, Jun 4, 2023 at 8:42 PM Ranier Vilela wrote: > >> Hi, >> >> Per Coverity. >> >> At function ExtendBufferedRelShared, has a always true test. >> eb.rel was dereferenced one line above, so in >> if (eb.rel) is always true. >> >>

Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

On Sun, Jun 4, 2023 at 8:42 PM Ranier Vilela wrote: > Hi, > > Per Coverity. > > At function ExtendBufferedRelShared, has a always true test. > eb.rel was dereferenced one line above, so in > if (eb.rel) is always true. > > I think it's worth removing the test, because Coverity raises dozens of >

Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

Hi, Per Coverity. At function ExtendBufferedRelShared, has a always true test. eb.rel was dereferenced one line above, so in if (eb.rel) is always true. I think it's worth removing the test, because Coverity raises dozens of alerts thinking eb.rel might be NULL. Besides, one less test is one les