On Mon, Feb 11, 2019 at 10:48 PM John Naylor <john.nay...@2ndquadrant.com> wrote: > > On 2/9/19, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Feb 5, 2019 at 3:25 PM John Naylor <john.nay...@2ndquadrant.com> > > wrote: > >> > >> On Tue, Feb 5, 2019 at 4:04 AM Amit Kapila <amit.kapil...@gmail.com> > >> wrote: > > This is certainly a good test w.r.t code coverage of new code, but I > > have few comments: > > 1. The size of records in test still depends on alignment (MAXALIGN). > > Though it doesn't seem to be a problematic case, I still suggest we > > can avoid using records whose size depends on alignment. If you > > change the schema as CREATE TABLE fsm_check_size (num1 int, num2 int, > > str text);, then you can avoid alignment related issues for the > > records being used in test. > > Done. > > > 2. > > +-- Fill most of the last block > > .. > > +-- Make sure records can go into any block but the last one > > .. > > +-- Insert large record and make sure it does not cause the relation to > > extend > > > > The comments in some part of the test seems too focussed towards the > > algorithm used for in-memory map. I think we can keep these if we > > want, but it is required to write a more generic comment stating what > > is the actual motive of additional tests (basically we are testing the > > functionality of in-memory map (LSM) for the heap, so we should write > > about it.). > > Done. >
Thanks, the modification looks good. I have slightly changed the commit message in the attached patch. I will spend some more time tomorrow morning on this and will commit unless I see any new problem. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
v3-0001-Add-more-tests-for-FSM.patch
Description: Binary data
