On Mon, Jul 20, 2026 at 10:56 AM Melanie Plageman <[email protected]> wrote: > > On Sun, Jul 19, 2026 at 10:34 PM Tom Lane <[email protected]> wrote: > > > > Melanie Plageman <[email protected]> writes: > > > I've committed this. > > > > Coverity complained about this patch: > > > > /srv/coverity/git/pgsql-git/postgresql/src/backend/access/heap/pruneheap.c: > > 951 in heap_page_fix_vm_corruption() > > 945 MarkBufferDirtyHint(prstate->buffer, true); > > 946 } > > 947 > > 948 if (do_clear_vm) > > 949 { > > 950 LockBuffer(prstate->vmbuffer, BUFFER_LOCK_EXCLUSIVE); > > >>> CID 1697133: Error handling issues (CHECKED_RETURN) > > >>> Calling "visibilitymap_clear" without checking return value (as is > > >>> done elsewhere 13 out of 14 times). > > 951 visibilitymap_clear(prstate->relation->rd_locator, > > prstate->block, > > 952 prstate->vmbuffer, > > 953 VISIBILITYMAP_VALID_BITS); > > 954 LockBuffer(prstate->vmbuffer, BUFFER_LOCK_UNLOCK); > > 955 prstate->old_vmbits = 0; > > 956 } > > > > I think it's right to complain --- if we check for failure everywhere > > else, why's it OK to not check here? If it is OK, a comment and an > > explicit cast to "(void)" would be appropriate. > > Got it. I didn't know we always checked return values everywhere in > the code base. > > In other cases where we call visibilitymap_clear(), we use the return > value to decide whether or not to emit WAL (because if the page wasn't > modified, we shouldn't). Not emitting WAL here is its own questionable > issue (historical, not something I introduced), but since we don't, I > didn't need the return value. I'll push a comment and cast next week > (I'm traveling this week and don't commit on the road if I can help > it).
K, I committed this to 19 and master. In backbranches we actually check the return value even less -- coverity was complaining because we started checking the return value more often and it went over some threshold. - Melanie
