Em sex., 26 de jun. de 2020 às 18:24, Tom Lane <t...@sss.pgh.pa.us> escreveu:
> Ranier Vilela <ranier...@gmail.com> writes: > > 1.Assertion check > > /* Caller messed up if we have neither a ready query nor held data. */ > > Assert(queryDesc || portal->holdStore); > > > But in release, if QueryDesc is NULL and portal->holdStore is NULL too, > > when Call PushActiveSnapshot *deference* NULL check can happen. > > > 2. if (portal->atEnd || count <= 0) is True > > No need to recheck count against FETCH_ALL. > > > Is it worth correcting them? > > No. > > The assertion already says that that's a case that cannot happen. > Or to look at it another way: if the case were to occur in a devel > build, you'd get a core dump at the assertion. If the case were > to occur in a production build, you'd get a core dump at the > dereference. Not much difference. Either way, it's a *caller* > bug, because the caller is supposed to make sure this cannot happen. > If we thought that it could possibly happen, we would use an ereport > but not an assertion; having both for the same condition is quite > misguided. > Ok, thats a job of Assertion. But I still worry that, in some rare cases, portal-> holdStore might be corrupted in some way and the function is called, causing a segmentation fault. > > (If Coverity is whining about this for you, there's something wrong > with your Coverity settings. In the project's instance, Coverity > accepts assertions as assertions.) > Probable, because reports this: CID 10127 (#2 of 2): Dereference after null check (FORWARD_NULL)8. var_deref_op: Dereferencing null pointer queryDesc. > > I'm unimpressed with the other proposed change too; it's making the logic > more complicated and fragile for a completely negligible "performance > gain". Moreover the compiler could probably make the same optimization. > Ok. Anyway, thank you for by responding, your observations are always valuable and help learn "the postgres way" to develop. It's not easy. best regards, Ranier Vilela
