Re: Refactoring of heapam code. - Mailing list pgsql-hackers

08.08.2016 12:43, Anastasia Lubennikova:
> 08.08.2016 03:51, Michael Paquier:
>> On Sat, Aug 6, 2016 at 2:56 AM, Alvaro Herrera 
>> <alvherre@2ndquadrant.com> wrote:
>>> Anastasia Lubennikova wrote:
>>>> So there is a couple of patches. They do not cover all mentioned 
>>>> problems,
>>>> but I'd like to get a feedback before continuing.
>>> I agree that we could improve things in this general area, but I do not
>>> endorse any particular changes you propose in these patches; I haven't
>>> reviewed your patches.
>> Well, I am interested in the topic. And just had a look at the 
>> patches proposed.
>>
>> + /*
>> + * Split PageGetItem into set of different macros
>> + * in order to make code more readable.
>> + */
>> +#define PageGetItemHeap(page, itemId) \
>> +( \
>> +   AssertMacro(PageIsValid(page)), \
>> +   AssertMacro(ItemIdHasStorage(itemId)), \
>> +   (HeapTupleHeader)(((char *)(page)) + ItemIdGetOffset(itemId)) \
>> +)
>> Placing into bufpage.h macros that are specific to heap and indexes is
>> not that much a good idea. I'd suggest having the heap ones in
>> heapam.h, and the index ones in a more generic place, as
>> src/access/common as already mentioned by Alvaro.
>>
>> +       onpage_tup = PageGetItemHeapHeaderOnly(page, newitemid);
>> Just PageGetItemHeapHeader would be fine.
>>
>> @@ -2324,7 +2087,6 @@ FreeBulkInsertState(BulkInsertState bistate)
>>      pfree(bistate);
>>   }
>>
>> -
>>   /*
>>    * heap_insert     - insert tuple into a heap
>> Those patches have some noise.
>>
>> Patch 0002 is doing two things: reorganizing the order of the routines
>> in heapam.c and move some routines from heapam.c to hio.c. Those two
>> could be separate patches/
>>
>> If the final result is to be able to extend custom AMs for tables, we
>> should have a structure like src/access/common/index.c,
>> src/access/common/table.c and src/access/common/relation.c for
>> example, and have headers dedicated to them. But yes, there is
>> definitely a lot of room for refactoring as we are aiming, at least I
>> guess so, at having more various code paths for access methods.
>
> Thank you for the review, I'll fix these problems in final version.
>
> Posting the first message I intended to raise the discussion. Patches
> were attached mainly to illustrate the problem and to be more concrete.
>
> Thank you for attention and feedback.
> Since there are no objections to the idea in general, I'll write an 
> exhaustive
> README about current state of the code and then propose API design
> to discuss details.
>
> Stay tuned.
>

Here is the promised design draft.
https://wiki.postgresql.org/wiki/HeapamRefactoring

Looking forward to your comments.

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company