mirror of
https://github.com/postgres/postgres.git
synced 2025-05-09 18:21:05 +03:00
Fix some coding issues in BRIN
Reported by David Rowley: variadic macros are a problem. Get rid of them using a trick suggested by Tom Lane: add extra parentheses where needed. In the future we might decide we don't need the calls at all and remove them, but it seems appropriate to keep them while this code is still new. Also from David Rowley: brininsert() was trying to use a variable before initializing it. Fix by moving the brin_form_tuple call (which initializes the variable) to within the locked section. Reported by Peter Eisentraut: can't use "new" as a struct member name, because C++ compilers will choke on it, as reported by cpluspluscheck.
This commit is contained in:
parent
926f5cea47
commit
b89ee54e20
@ -247,11 +247,9 @@ brininsert(PG_FUNCTION_ARGS)
|
|||||||
* the same page though, so downstream we must be prepared to cope
|
* the same page though, so downstream we must be prepared to cope
|
||||||
* if this turns out to not be possible after all.
|
* if this turns out to not be possible after all.
|
||||||
*/
|
*/
|
||||||
samepage = brin_can_do_samepage_update(buf, origsz, newsz);
|
|
||||||
|
|
||||||
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
|
|
||||||
|
|
||||||
newtup = brin_form_tuple(bdesc, heapBlk, dtup, &newsz);
|
newtup = brin_form_tuple(bdesc, heapBlk, dtup, &newsz);
|
||||||
|
samepage = brin_can_do_samepage_update(buf, origsz, newsz);
|
||||||
|
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Try to update the tuple. If this doesn't work for whatever
|
* Try to update the tuple. If this doesn't work for whatever
|
||||||
@ -589,9 +587,10 @@ brinbuildCallback(Relation index,
|
|||||||
while (thisblock > state->bs_currRangeStart + state->bs_pagesPerRange - 1)
|
while (thisblock > state->bs_currRangeStart + state->bs_pagesPerRange - 1)
|
||||||
{
|
{
|
||||||
|
|
||||||
BRIN_elog(DEBUG2, "brinbuildCallback: completed a range: %u--%u",
|
BRIN_elog((DEBUG2,
|
||||||
state->bs_currRangeStart,
|
"brinbuildCallback: completed a range: %u--%u",
|
||||||
state->bs_currRangeStart + state->bs_pagesPerRange);
|
state->bs_currRangeStart,
|
||||||
|
state->bs_currRangeStart + state->bs_pagesPerRange));
|
||||||
|
|
||||||
/* create the index tuple and insert it */
|
/* create the index tuple and insert it */
|
||||||
form_and_insert_tuple(state);
|
form_and_insert_tuple(state);
|
||||||
|
@ -216,12 +216,12 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
|
|||||||
|
|
||||||
info = XLOG_BRIN_UPDATE | (extended ? XLOG_BRIN_INIT_PAGE : 0);
|
info = XLOG_BRIN_UPDATE | (extended ? XLOG_BRIN_INIT_PAGE : 0);
|
||||||
|
|
||||||
xlrec.new.node = idxrel->rd_node;
|
xlrec.insert.node = idxrel->rd_node;
|
||||||
ItemPointerSet(&xlrec.new.tid, BufferGetBlockNumber(newbuf), newoff);
|
ItemPointerSet(&xlrec.insert.tid, BufferGetBlockNumber(newbuf), newoff);
|
||||||
xlrec.new.heapBlk = heapBlk;
|
xlrec.insert.heapBlk = heapBlk;
|
||||||
xlrec.new.tuplen = newsz;
|
xlrec.insert.tuplen = newsz;
|
||||||
xlrec.new.revmapBlk = BufferGetBlockNumber(revmapbuf);
|
xlrec.insert.revmapBlk = BufferGetBlockNumber(revmapbuf);
|
||||||
xlrec.new.pagesPerRange = pagesPerRange;
|
xlrec.insert.pagesPerRange = pagesPerRange;
|
||||||
ItemPointerSet(&xlrec.oldtid, BufferGetBlockNumber(oldbuf), oldoff);
|
ItemPointerSet(&xlrec.oldtid, BufferGetBlockNumber(oldbuf), oldoff);
|
||||||
|
|
||||||
rdata[0].data = (char *) &xlrec;
|
rdata[0].data = (char *) &xlrec;
|
||||||
@ -342,8 +342,8 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
|
|||||||
elog(ERROR, "could not insert new index tuple to page");
|
elog(ERROR, "could not insert new index tuple to page");
|
||||||
MarkBufferDirty(*buffer);
|
MarkBufferDirty(*buffer);
|
||||||
|
|
||||||
BRIN_elog(DEBUG2, "inserted tuple (%u,%u) for range starting at %u",
|
BRIN_elog((DEBUG2, "inserted tuple (%u,%u) for range starting at %u",
|
||||||
blk, off, heapBlk);
|
blk, off, heapBlk));
|
||||||
|
|
||||||
ItemPointerSet(&tid, blk, off);
|
ItemPointerSet(&tid, blk, off);
|
||||||
brinSetHeapBlockItemptr(revmapbuf, pagesPerRange, heapBlk, tid);
|
brinSetHeapBlockItemptr(revmapbuf, pagesPerRange, heapBlk, tid);
|
||||||
@ -593,8 +593,8 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
|
|||||||
newblk = BufferGetBlockNumber(buf);
|
newblk = BufferGetBlockNumber(buf);
|
||||||
*was_extended = extended = true;
|
*was_extended = extended = true;
|
||||||
|
|
||||||
BRIN_elog(DEBUG2, "brin_getinsertbuffer: extending to page %u",
|
BRIN_elog((DEBUG2, "brin_getinsertbuffer: extending to page %u",
|
||||||
BufferGetBlockNumber(buf));
|
BufferGetBlockNumber(buf)));
|
||||||
}
|
}
|
||||||
else if (newblk == oldblk)
|
else if (newblk == oldblk)
|
||||||
{
|
{
|
||||||
|
@ -331,10 +331,6 @@ revmap_get_buffer(BrinRevmap *revmap, BlockNumber heapBlk)
|
|||||||
Assert(mapBlk != BRIN_METAPAGE_BLKNO &&
|
Assert(mapBlk != BRIN_METAPAGE_BLKNO &&
|
||||||
mapBlk <= revmap->rm_lastRevmapPage);
|
mapBlk <= revmap->rm_lastRevmapPage);
|
||||||
|
|
||||||
BRIN_elog(DEBUG2, "getting revmap page for logical page %lu (physical %u) for heap %u",
|
|
||||||
HEAPBLK_TO_REVMAP_BLK(revmap->rm_pagesPerRange, heapBlk),
|
|
||||||
mapBlk, heapBlk);
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Obtain the buffer from which we need to read. If we already have the
|
* Obtain the buffer from which we need to read. If we already have the
|
||||||
* correct buffer in our access struct, use that; otherwise, release that,
|
* correct buffer in our access struct, use that; otherwise, release that,
|
||||||
|
@ -144,7 +144,7 @@ brin_xlog_update(XLogRecPtr lsn, XLogRecord *record)
|
|||||||
|
|
||||||
/* First remove the old tuple */
|
/* First remove the old tuple */
|
||||||
blkno = ItemPointerGetBlockNumber(&(xlrec->oldtid));
|
blkno = ItemPointerGetBlockNumber(&(xlrec->oldtid));
|
||||||
action = XLogReadBufferForRedo(lsn, record, 2, xlrec->new.node,
|
action = XLogReadBufferForRedo(lsn, record, 2, xlrec->insert.node,
|
||||||
blkno, &buffer);
|
blkno, &buffer);
|
||||||
if (action == BLK_NEEDS_REDO)
|
if (action == BLK_NEEDS_REDO)
|
||||||
{
|
{
|
||||||
@ -164,7 +164,7 @@ brin_xlog_update(XLogRecPtr lsn, XLogRecord *record)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Then insert the new tuple and update revmap, like in an insertion. */
|
/* Then insert the new tuple and update revmap, like in an insertion. */
|
||||||
brin_xlog_insert_update(lsn, record, &xlrec->new, newtup);
|
brin_xlog_insert_update(lsn, record, &xlrec->insert, newtup);
|
||||||
|
|
||||||
if (BufferIsValid(buffer))
|
if (BufferIsValid(buffer))
|
||||||
UnlockReleaseBuffer(buffer);
|
UnlockReleaseBuffer(buffer);
|
||||||
|
@ -49,14 +49,14 @@ brin_desc(StringInfo buf, XLogRecord *record)
|
|||||||
xl_brin_update *xlrec = (xl_brin_update *) rec;
|
xl_brin_update *xlrec = (xl_brin_update *) rec;
|
||||||
|
|
||||||
appendStringInfo(buf, "rel %u/%u/%u heapBlk %u revmapBlk %u pagesPerRange %u old TID (%u,%u) TID (%u,%u)",
|
appendStringInfo(buf, "rel %u/%u/%u heapBlk %u revmapBlk %u pagesPerRange %u old TID (%u,%u) TID (%u,%u)",
|
||||||
xlrec->new.node.spcNode, xlrec->new.node.dbNode,
|
xlrec->insert.node.spcNode, xlrec->insert.node.dbNode,
|
||||||
xlrec->new.node.relNode,
|
xlrec->insert.node.relNode,
|
||||||
xlrec->new.heapBlk, xlrec->new.revmapBlk,
|
xlrec->insert.heapBlk, xlrec->insert.revmapBlk,
|
||||||
xlrec->new.pagesPerRange,
|
xlrec->insert.pagesPerRange,
|
||||||
ItemPointerGetBlockNumber(&xlrec->oldtid),
|
ItemPointerGetBlockNumber(&xlrec->oldtid),
|
||||||
ItemPointerGetOffsetNumber(&xlrec->oldtid),
|
ItemPointerGetOffsetNumber(&xlrec->oldtid),
|
||||||
ItemPointerGetBlockNumber(&xlrec->new.tid),
|
ItemPointerGetBlockNumber(&xlrec->insert.tid),
|
||||||
ItemPointerGetOffsetNumber(&xlrec->new.tid));
|
ItemPointerGetOffsetNumber(&xlrec->insert.tid));
|
||||||
}
|
}
|
||||||
else if (info == XLOG_BRIN_SAMEPAGE_UPDATE)
|
else if (info == XLOG_BRIN_SAMEPAGE_UPDATE)
|
||||||
{
|
{
|
||||||
|
@ -72,13 +72,12 @@ typedef struct BrinDesc
|
|||||||
#define BRIN_PROCNUM_UNION 4
|
#define BRIN_PROCNUM_UNION 4
|
||||||
/* procedure numbers up to 10 are reserved for BRIN future expansion */
|
/* procedure numbers up to 10 are reserved for BRIN future expansion */
|
||||||
|
|
||||||
#define BRIN_DEBUG
|
#undef BRIN_DEBUG
|
||||||
|
|
||||||
/* we allow debug if using GCC; otherwise don't bother */
|
#ifdef BRIN_DEBUG
|
||||||
#if defined(BRIN_DEBUG) && defined(__GNUC__)
|
#define BRIN_elog(args) elog args
|
||||||
#define BRIN_elog(level, ...) elog(level, __VA_ARGS__)
|
|
||||||
#else
|
#else
|
||||||
#define BRIN_elog(a) void(0)
|
#define BRIN_elog(args) ((void) 0)
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
/* brin.c */
|
/* brin.c */
|
||||||
|
@ -76,10 +76,10 @@ typedef struct xl_brin_insert
|
|||||||
typedef struct xl_brin_update
|
typedef struct xl_brin_update
|
||||||
{
|
{
|
||||||
ItemPointerData oldtid;
|
ItemPointerData oldtid;
|
||||||
xl_brin_insert new;
|
xl_brin_insert insert;
|
||||||
} xl_brin_update;
|
} xl_brin_update;
|
||||||
|
|
||||||
#define SizeOfBrinUpdate (offsetof(xl_brin_update, new) + SizeOfBrinInsert)
|
#define SizeOfBrinUpdate (offsetof(xl_brin_update, insert) + SizeOfBrinInsert)
|
||||||
|
|
||||||
/* This is what we need to know about a BRIN tuple samepage update */
|
/* This is what we need to know about a BRIN tuple samepage update */
|
||||||
typedef struct xl_brin_samepage_update
|
typedef struct xl_brin_samepage_update
|
||||||
|
Loading…
x
Reference in New Issue
Block a user