On Sat, Nov 8, 2014 at 1:26 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > > > I just pushed this, after some more minor tweaks. Thanks, and please do > continue testing! >
Few typo's and few questions
1. * range. Need to an extra flag in mmtuples for that.
*/
Datum
brinbulkdelete(PG_FUNCTION_ARGS)
Isn't the part of comment referring *mmtuples* require some change,
as I think mmtuples was used in initial version of patch.
2.
/* ---------------
* mt_info is laid out in the following fashion:
*
* 7th (high)
bit: has nulls
* 6th bit: is placeholder tuple
* 5th bit: unused
* 4-0 bit: offset of data
* ---------------
*/
uint8bt_info;
} BrinTuple;
Here in comments, bt_info is referred as mt_info.
3.
/*
* t_info manipulation macros
*/
#define BRIN_OFFSET_MASK0x1F
I think in above comment it should be bt_info, rather than t_info.
4.
static void
revmap_physical_extend(BrinRevmap *revmap)
{
..
..
START_CRIT_SECTION();
/* the rm_tids array is initialized to all invalid by PageInit */
brin_page_init(page, BRIN_PAGETYPE_REVMAP);
MarkBufferDirty(buf);
metadata->lastRevmapPage = mapBlk;
MarkBufferDirty(revmap->rm_metaBuf);
..
}
Can't we update revmap->rm_lastRevmapPage along with metadata->lastRevmap?
5.
typedef struct BrinMemTuple
{
boolbt_placeholder; /* this is a placeholder tuple */
BlockNumber bt_blkno;/* heap blkno that the tuple is for */
MemoryContext bt_context;/*
memcxt holding the dt_column values */
..
}
How is this memory context getting used?
I could see that this is used brin_deform_tuple() which gets called from
3 other places in core code bringetbitmap(), brininsert() and union_tuples()
and in all the 3 places there is already another temporaray memory context
used to avoid any form of memory leaks.
6.
Is there anyway to force brin index to be off, if not, then do we need it
