On Fri, Aug 23, 2019 at 7:05 AM Ashwin Agrawal <aagra...@pivotal.io> wrote: > > On Thu, Aug 22, 2019 at 1:14 AM Heikki Linnakangas <hlinn...@iki.fi> wrote: >> >> >> The patch also includes a little unit test module to test this without >> creating a 16 TB table. A whole new test module seems a bit like >> overkill just for this, but clearly we were missing test coverage here. >> And it will come handy, if we want to invent a new better posting list >> format in the future. Thoughts on whether to include the test module or not? > > > I like the test as importantly adds missing coverage. Also, really simplifies > validation effort if required to make change in this area anytime in future. > So, I would +1 keeping the same.
I'd +1 too. It's valuable to test hard-to-reproduce case. I often want to do such unit tests with more cheaper costs, though. BTW it's not related to this patch but I got confused that where "17 bits" of the following paragraph in ginpostinglist.c comes from. If we use only 43 bit out of 64-bit unsigned integer we have 21 bits left. * For encoding purposes, item pointers are represented as 64-bit unsigned * integers. The lowest 11 bits represent the offset number, and the next * lowest 32 bits are the block number. That leaves 17 bits unused, i.e. * only 43 low bits are used. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
