On 2017-12-14 21:06:34 +1300, Thomas Munro wrote: > On Thu, Dec 14, 2017 at 11:45 AM, Andres Freund <and...@anarazel.de> wrote: > > + if (accessor->write_pointer + > > accessor->sts->meta_data_size >= > > + accessor->write_end) > > + elog(ERROR, "meta-data too long"); > > > > That seems more like an Assert than a proper elog? Given that we're > > calculating size just a few lines above... > > It's an error because the logic is not smart enough to split the > optional meta-data and tuple size over multiple chunks. I have added > comments there to explain.
I don't see how that requires it to be an elog rather than an assert. As far as I can tell this is only reachable if meta_data_size + sizeof(uint32) >= STS_CHUNK_DATA_SIZE. For anything smaller the sts_flush_chunk() a few lines above would have started a new chunk. IOW, we should detect this at sts_initialize() time, elog there, and Assert() out in puttuple. I've changed the code like that, fixed my own < vs >= confusion during the conversion, fixed a typo, and pushed. If you're not ok with that change, we can easily whack this around. I've not changed this, but I wonder whether we should rename sts_puttuple() to sts_put_tuple(), for consistency with sts_read_tuple(). Or the other way round. Or... Greetings, Andres Freund
