Tom Lane wrote:
> I'm noticing that the latest pgindent run has frequently rejustified
> block comments to end in column 80 or 81, causing them to wrap in an
> ugly way (at least in emacs). I thought the agreement was to limit
> lines to 79 chars max?
>
> For one example see lines 475 ff in /src/backend/access/nbtree/nbtpage.c
> --- the first lines of two successive paragraphs in the comment have
> been made too long, which they were not before.
>
> I'm not sure about this offhand, but I think that all the cases I've
> seen have involved first lines of paragraphs inside block comments.
Good point. I see the maximum length changed in this commit:
revision 1.70
date: 2004/10/02 01:10:58; author: momjian; state: Exp; lines: +1 -1
Update length from 75 to 79.
We were discussing some pgindent issues at that time on hackers, but I
don't see any complaints about the length, so I am unsure why I modified
it, but perhaps I received a private communication asking why it wasn't
79.
Anyway, I have updated the script to be 78, and attached is a diff
against nbpage.c, but I have not applied a change to that file.
Would you like another pgindent run with the new value of 78? Should be
run on CVS HEAD only or 8.0.X too?
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: nbtpage.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/access/nbtree/nbtpage.c,v
retrieving revision 1.89
diff -c -r1.89 nbtpage.c
*** nbtpage.c 6 Nov 2005 19:29:00 -0000 1.89
--- nbtpage.c 7 Nov 2005 22:53:41 -0000
***************
*** 205,223 ****
if (metad->btm_root != P_NONE)
{
/*
! * Metadata initialized by someone else. In order to
guarantee no
! * deadlocks, we have to release the metadata page and
start all
! * over again. (Is that really true? But it's hardly
worth trying
! * to optimize this case.)
*/
_bt_relbuf(rel, metabuf);
return _bt_getroot(rel, access);
}
/*
! * Get, initialize, write, and leave a lock of the appropriate
type on
! * the new root page. Since this is the first page in the
tree, it's
! * a leaf as well as the root.
*/
rootbuf = _bt_getbuf(rel, P_NEW, BT_WRITE);
rootblkno = BufferGetBlockNumber(rootbuf);
--- 205,223 ----
if (metad->btm_root != P_NONE)
{
/*
! * Metadata initialized by someone else. In order to
guarantee
! * no deadlocks, we have to release the metadata page
and start
! * all over again. (Is that really true? But it's
hardly worth
! * trying to optimize this case.)
*/
_bt_relbuf(rel, metabuf);
return _bt_getroot(rel, access);
}
/*
! * Get, initialize, write, and leave a lock of the appropriate
type
! * on the new root page. Since this is the first page in the
tree,
! * it's a leaf as well as the root.
*/
rootbuf = _bt_getbuf(rel, P_NEW, BT_WRITE);
rootblkno = BufferGetBlockNumber(rootbuf);
***************
*** 412,427 ****
Page page = BufferGetPage(buf);
/*
! * ReadBuffer verifies that every newly-read page passes
PageHeaderIsValid,
! * which means it either contains a reasonably sane page header or is
! * all-zero. We have to defend against the all-zero case, however.
*/
if (PageIsNew(page))
ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED),
! errmsg("index \"%s\" contains unexpected zero
page at block %u",
! RelationGetRelationName(rel),
! BufferGetBlockNumber(buf)),
errhint("Please REINDEX it.")));
/*
--- 412,428 ----
Page page = BufferGetPage(buf);
/*
! * ReadBuffer verifies that every newly-read page passes
! * PageHeaderIsValid, which means it either contains a reasonably sane
! * page header or is all-zero. We have to defend against the all-zero
! * case, however.
*/
if (PageIsNew(page))
ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED),
! errmsg("index \"%s\" contains unexpected zero page at
block %u",
! RelationGetRelationName(rel),
! BufferGetBlockNumber(buf)),
errhint("Please REINDEX it.")));
/*
***************
*** 440,446 ****
/*
* _bt_getbuf() -- Get a buffer by block number for read or write.
*
! * blkno == P_NEW means to get an unallocated index page. The page
* will be initialized before returning it.
*
* When this routine returns, the appropriate lock is set on the
--- 441,447 ----
/*
* _bt_getbuf() -- Get a buffer by block number for read or write.
*
! * blkno == P_NEW means to get an unallocated index page. The page
* will be initialized before returning it.
*
* When this routine returns, the appropriate lock is set on the
***************
*** 480,495 ****
* it, or even worse our own caller does, we could deadlock.
(The
* own-caller scenario is actually not improbable. Consider an
index
* on a serial or timestamp column. Nearly all splits will be
at the
! * rightmost page, so it's entirely likely that _bt_split will
call us
! * while holding a lock on the page most recently acquired from
FSM.
! * A VACUUM running concurrently with the previous split could
well
! * have placed that page back in FSM.)
*
! * To get around that, we ask for only a conditional lock on
the reported
! * page. If we fail, then someone else is using the page, and
we may
! * reasonably assume it's not free. (If we happen to be wrong,
the
! * worst consequence is the page will be lost to use till the
next
! * VACUUM, which is no big problem.)
*/
for (;;)
{
--- 481,496 ----
* it, or even worse our own caller does, we could deadlock.
(The
* own-caller scenario is actually not improbable. Consider an
index
* on a serial or timestamp column. Nearly all splits will be
at the
! * rightmost page, so it's entirely likely that _bt_split will
call
! * us while holding a lock on the page most recently acquired
from
! * FSM. A VACUUM running concurrently with the previous split
could
! * well have placed that page back in FSM.)
*
! * To get around that, we ask for only a conditional lock on the
! * reported page. If we fail, then someone else is using the
page,
! * and we may reasonably assume it's not free. (If we happen
to be
! * wrong, the worst consequence is the page will be lost to use
till
! * the next VACUUM, which is no big problem.)
*/
for (;;)
{
***************
*** 649,658 ****
BTPageOpaque opaque;
/*
! * It's possible to find an all-zeroes page in an index --- for
example, a
! * backend might successfully extend the relation one page and then
crash
! * before it is able to make a WAL entry for adding the page. If we
find a
! * zeroed page then reclaim it.
*/
if (PageIsNew(page))
return true;
--- 650,659 ----
BTPageOpaque opaque;
/*
! * It's possible to find an all-zeroes page in an index --- for example,
! * a backend might successfully extend the relation one page and then
! * crash before it is able to make a WAL entry for adding the page. If
we
! * find a zeroed page then reclaim it.
*/
if (PageIsNew(page))
return true;
***************
*** 782,789 ****
BTPageOpaque opaque;
/*
! * We can never delete rightmost pages nor root pages. While at it,
check
! * that page is not already deleted and is empty.
*/
page = BufferGetPage(buf);
opaque = (BTPageOpaque) PageGetSpecialPointer(page);
--- 783,790 ----
BTPageOpaque opaque;
/*
! * We can never delete rightmost pages nor root pages. While at it,
! * check that page is not already deleted and is empty.
*/
page = BufferGetPage(buf);
opaque = (BTPageOpaque) PageGetSpecialPointer(page);
***************
*** 808,815 ****
* We need to get an approximate pointer to the page's parent page. Use
* the standard search mechanism to search for the page's high key; this
* will give us a link to either the current parent or someplace to its
! * left (if there are multiple equal high keys). To avoid deadlocks,
we'd
! * better drop the target page lock first.
*/
_bt_relbuf(rel, buf);
/* we need a scan key to do our search, so build one */
--- 809,816 ----
* We need to get an approximate pointer to the page's parent page. Use
* the standard search mechanism to search for the page's high key; this
* will give us a link to either the current parent or someplace to its
! * left (if there are multiple equal high keys). To avoid deadlocks,
! * we'd better drop the target page lock first.
*/
_bt_relbuf(rel, buf);
/* we need a scan key to do our search, so build one */
***************
*** 843,851 ****
* page. The sibling that was current a moment ago could have split, so
* we may have to move right. This search could fail if either the
* sibling or the target page was deleted by someone else meanwhile; if
! * so, give up. (Right now, that should never happen, since page
deletion
! * is only done in VACUUM and there shouldn't be multiple VACUUMs
! * concurrently on the same table.)
*/
if (leftsib != P_NONE)
{
--- 844,852 ----
* page. The sibling that was current a moment ago could have split, so
* we may have to move right. This search could fail if either the
* sibling or the target page was deleted by someone else meanwhile; if
! * so, give up. (Right now, that should never happen, since page
! * deletion is only done in VACUUM and there shouldn't be multiple
! * VACUUMs concurrently on the same table.)
*/
if (leftsib != P_NONE)
{
***************
*** 872,880 ****
lbuf = InvalidBuffer;
/*
! * Next write-lock the target page itself. It should be okay to
take just
! * a write lock not a superexclusive lock, since no scans would stop on
an
! * empty page.
*/
buf = _bt_getbuf(rel, target, BT_WRITE);
page = BufferGetPage(buf);
--- 873,881 ----
lbuf = InvalidBuffer;
/*
! * Next write-lock the target page itself. It should be okay to
take
! * just a write lock not a superexclusive lock, since no scans would
stop
! * on an empty page.
*/
buf = _bt_getbuf(rel, target, BT_WRITE);
page = BufferGetPage(buf);
***************
*** 904,911 ****
rbuf = _bt_getbuf(rel, rightsib, BT_WRITE);
/*
! * Next find and write-lock the current parent of the target page. This
is
! * essentially the same as the corresponding step of splitting.
*/
ItemPointerSet(&(stack->bts_btitem.bti_itup.t_tid),
target, P_HIKEY);
--- 905,912 ----
rbuf = _bt_getbuf(rel, rightsib, BT_WRITE);
/*
! * Next find and write-lock the current parent of the target page. This
! * is essentially the same as the corresponding step of splitting.
*/
ItemPointerSet(&(stack->bts_btitem.bti_itup.t_tid),
target, P_HIKEY);
***************
*** 949,957 ****
/*
* If we are deleting the next-to-last page on the target's level, then
! * the rightsib is a candidate to become the new fast root. (In theory,
it
! * might be possible to push the fast root even further down, but the
odds
! * of doing so are slim, and the locking considerations daunting.)
*
* We can safely acquire a lock on the metapage here --- see comments
for
* _bt_newroot().
--- 950,958 ----
/*
* If we are deleting the next-to-last page on the target's level, then
! * the rightsib is a candidate to become the new fast root. (In theory,
! * it might be possible to push the fast root even further down, but the
! * odds of doing so are slim, and the locking considerations daunting.)
*
* We can safely acquire a lock on the metapage here --- see comments
for
* _bt_newroot().
***************
*** 992,999 ****
/*
* Update parent. The normal case is a tad tricky because we want to
* delete the target's downlink and the *following* key. Easiest way is
! * to copy the right sibling's downlink over the target downlink, and
then
! * delete the following item.
*/
page = BufferGetPage(pbuf);
opaque = (BTPageOpaque) PageGetSpecialPointer(page);
--- 993,1000 ----
/*
* Update parent. The normal case is a tad tricky because we want to
* delete the target's downlink and the *following* key. Easiest way is
! * to copy the right sibling's downlink over the target downlink, and
! * then delete the following item.
*/
page = BufferGetPage(pbuf);
opaque = (BTPageOpaque) PageGetSpecialPointer(page);
***************
*** 1154,1162 ****
/*
* If parent became half dead, recurse to try to delete it. Otherwise,
if
! * right sibling is empty and is now the last child of the parent,
recurse
! * to try to delete it. (These cases cannot apply at the same time,
! * though the second case might itself recurse to the first.)
*/
if (parent_half_dead)
{
--- 1155,1163 ----
/*
* If parent became half dead, recurse to try to delete it. Otherwise,
if
! * right sibling is empty and is now the last child of the parent,
! * recurse to try to delete it. (These cases cannot apply at the same
! * time, though the second case might itself recurse to the first.)
*/
if (parent_half_dead)
{
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
