Re: Confine vacuum skip logic to lazy_scan_skip

Andres Freund <andres(at)anarazel(dot)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