On Sat, Aug 6, 2016 at 2:56 AM, Alvaro Herrera <alvhe...@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. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers