Hello, I looked into the patch and have some comments. >From the restriction of the time for this rather big patch, please excuse that these comments are on a part of it. Others will follow in few days.
==== heaptuple.c noncachegetattr(_with_len): - att_getlength should do strlen as worst case or VARSIZE_ANY which is heavier than doing one comparizon, so I recommend to add 'if (len)' as the restriction for doing this, and give NULL as &len to nocachegetattr_with_len in nocachegetattr. heap_attr_get_length_and_check_equals: - Size seems to be used conventionary as the type for memory object length, so it might be better using Size instead of int32 as the type for *tup[12]_attr_len in parameter. - This function returns always false for attrnum <= 0 as whole tuple or some system attrs comparison regardless of the real result, which is a bit different from the anticipation which the name gives. If you need to keep this optimization, it should have the name more specific to the purpose. haap_delta_encode: - Some misleading variable names (like match_not_found), some reatitions of similiar codelets (att_align_pointer, pglz_out_tag), misleading slight difference of the meanings of variables of similar names(old_off and new_off and the similar pairs), and bit tricky use of pglz_out_add and pglz_out_tag with length = 0. These are welcome to be modified for better readability. ==== heapam.c fastgetattr_with_len - Missing left paren in the line 867 ('nocachegetattr_with_len(tup)...') - Missing enclosing paren in heapam.c:879 (len, only on style) - Allowing len = NULL will be good for better performance, like noncachegetattr. fastgetattr - I suppose that the coding covension here is that macro and alternative c-code are expected to be look similar. fastgetattr looks quite differ to corresponding macro. ... regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers