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. >
Oops, on again carefully studying the test, I realized my above comment was wrong. Let me explain with a test this time: CREATE TABLE fsm_check_size (num int, str text); INSERT INTO fsm_check_size SELECT i, rpad('', 1024, 'a') FROM generate_series(1,3) i; So here you are inserting 4-byte integer and 1024-bytes variable length record. So the tuple length will be tuple_header (24-bytes) + 4-bytes for integer + 4-bytes header for variable length data + 1024 bytes of actual data. So, the length will be 1056 which is already MAXALIGN. I took the new comments added in your latest version of the patch and added them to the previous version of the patch. Kindly see if I have not missed anything while merging the patch-versions? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
v4-0001-Add-more-tests-for-FSM.patch
Description: Binary data