Re: Confine vacuum skip logic to lazy_scan_skip - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Confine vacuum skip logic to lazy_scan_skip
Date
Msg-id 2600078.1740679727@sss.pgh.pa.us
Whole thread Raw
In response to Re: Confine vacuum skip logic to lazy_scan_skip  (Andres Freund <andres@anarazel.de>)
Responses Re: Confine vacuum skip logic to lazy_scan_skip
List pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> Ah, no, it isn't. But I still think the coverity alert and the patch don't
> make sense, as per the below:

Coverity's alert makes perfect sense if you posit that Coverity
doesn't assume that this read_stream_next_buffer call will
only be applied to a stream that has per_buffer_data_size > 0.
(Even if it did understand that, I wouldn't assume that it's
smart enough to see that the fast path will never be taken.)

I wonder if it'd be a good idea to add something like

        Assert(stream->distance == 1);
        Assert(stream->pending_read_nblocks == 0);
        Assert(stream->per_buffer_data_size == 0);
+        Assert(per_buffer_data == NULL);

in read_stream_next_buffer.  I doubt that this will shut Coverity
up, but it would help to catch caller coding errors, i.e. passing
a per_buffer_data pointer when there's no per-buffer data.

On the whole I doubt we can get rid of this warning without some
significant redesign of the read_stream API, and I don't think
it's worth the trouble.  Coverity is a tool not a requirement.
I'm content to just dismiss the warning.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: Re: Confine vacuum skip logic to lazy_scan_skip
Next
From: "David G. Johnston"
Date:
Subject: Re: Document How Commit Handles Aborted Transactions