On 07/11/2012 05:45 AM, Tom Lane wrote:
> I'm also inclined to think that the "while (stack)" coding of the rest
> of it is wrong, misleading, or both, on precisely the same grounds: if
> that loop ever did fall out at the test, the function would have failed
> to honor its contract. The only correct exit points are the "return"s
> in the middle.
I came to the same conclusion, yes. Looks like the additional asserts in
the attached patch all hold true.
As another minor improvement, it doesn't seem necessary to repeatedly
set the rootBlkno.
Regards
Markus Wanner
#
# old_revision [d90761edf47c6543d4686a76baa0b4e2a7ed113b]
#
# patch "src/backend/access/gin/ginbtree.c"
# from [2d3e63387737b4034fc25ca3cb128d9ac57f4f01]
# to [62efe71b8e3429fc1bf2d2742f8d2fa69f2f4de6]
#
============================================================
*** src/backend/access/gin/ginbtree.c 2d3e63387737b4034fc25ca3cb128d9ac57f4f01
--- src/backend/access/gin/ginbtree.c 62efe71b8e3429fc1bf2d2742f8d2fa69f2f4de6
*************** ginInsertValue(GinBtree btree, GinBtreeS
*** 276,294 ****
ginInsertValue(GinBtree btree, GinBtreeStack *stack, GinStatsData *buildStats)
{
GinBtreeStack *parent = stack;
! BlockNumber rootBlkno = InvalidBuffer;
Page page,
rpage,
lpage;
/* remember root BlockNumber */
! while (parent)
{
rootBlkno = parent->blkno;
parent = parent->parent;
! }
! while (stack)
{
XLogRecData *rdata;
BlockNumber savedRightLink;
--- 276,298 ----
ginInsertValue(GinBtree btree, GinBtreeStack *stack, GinStatsData *buildStats)
{
GinBtreeStack *parent = stack;
! BlockNumber rootBlkno;
Page page,
rpage,
lpage;
/* remember root BlockNumber */
! Assert(stack != NULL);
! parent = stack;
! do
{
rootBlkno = parent->blkno;
parent = parent->parent;
! } while (parent);
! Assert(BlockNumberIsValid(rootBlkno));
!
! for (;;)
{
XLogRecData *rdata;
BlockNumber savedRightLink;
*************** ginInsertValue(GinBtree btree, GinBtreeS
*** 469,474 ****
--- 473,479 ----
UnlockReleaseBuffer(stack->buffer);
pfree(stack);
+ Assert(parent != NULL); /* parent == NULL case is handled above */
stack = parent;
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
