This patch cleans up some code in the smgr and elsewhere:
- Update comment in IsReservedName() to the present day, remove
"ugly coding for speed" as this is no longer a performance
critical function
- Improve some variable & function names in commands/vacuum.c. I
was planning to rewrite this to avoid lappend(), but since I
still intend to do the list rewrite, there's no need for that.
- Update some smgr comments which seemed to imply that we still
forced all dirty pages to disk at commit-time.
- Replace some #ifdef DIAGNOSTIC code with assertions.
- Make the distinction between OS-level file descriptors and
virtual file descriptors a little clearer in a few comments
- Other minor comment improvements in the smgr code
Unless anyone objects, I intend to apply this code in about 48 hours.
-Neil
Index: src/backend/catalog/catalog.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql-server/src/backend/catalog/catalog.c,v
retrieving revision 1.50
diff -c -r1.50 catalog.c
*** src/backend/catalog/catalog.c 29 Nov 2003 19:51:42 -0000 1.50
--- src/backend/catalog/catalog.c 1 Jan 2004 22:23:03 -0000
***************
*** 160,182 ****
return namespaceId == PG_TOAST_NAMESPACE;
}
-
/*
* IsReservedName
* True iff name starts with the pg_ prefix.
*
! * For some classes of objects, the prefix pg_ is reserved
! * for system objects only.
*/
bool
IsReservedName(const char *name)
{
! /* ugly coding for speed */
! return (name[0] == 'p' &&
! name[1] == 'g' &&
! name[2] == '_');
}
-
/*
* newoid - returns a unique identifier across all catalogs.
--- 160,178 ----
return namespaceId == PG_TOAST_NAMESPACE;
}
/*
* IsReservedName
* True iff name starts with the pg_ prefix.
*
! * For some classes of objects, the prefix pg_ is reserved for
! * system objects only. As of 7.3, this is now only true for
! * schema names.
*/
bool
IsReservedName(const char *name)
{
! return strncmp(name, "pg_", 3);
}
/*
* newoid - returns a unique identifier across all catalogs.
Index: src/backend/commands/analyze.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql-server/src/backend/commands/analyze.c,v
retrieving revision 1.65
diff -c -r1.65 analyze.c
*** src/backend/commands/analyze.c 29 Nov 2003 19:51:47 -0000 1.65
--- src/backend/commands/analyze.c 1 Jan 2004 22:27:44 -0000
***************
*** 204,211 ****
}
/*
! * Check that it's a plain table; we used to do this in getrels() but
! * seems safer to check after we've locked the relation.
*/
if (onerel->rd_rel->relkind != RELKIND_RELATION)
{
--- 204,212 ----
}
/*
! * Check that it's a plain table; we used to do this in
! * get_rel_oids() but seems safer to check after we've locked the
! * relation.
*/
if (onerel->rd_rel->relkind != RELKIND_RELATION)
{
***************
*** 541,551 ****
*totalrows = (double) numrows;
ereport(elevel,
! (errmsg("\"%s\": %u pages, %d rows sampled, %.0f estimated total rows",
! RelationGetRelationName(onerel),
! onerel->rd_nblocks, numrows, *totalrows)));
!
! return numrows;
}
/*
--- 542,552 ----
*totalrows = (double) numrows;
ereport(elevel,
! (errmsg("\"%s\": %u pages, %d rows sampled, %.0f estimated total rows",
! RelationGetRelationName(onerel),
! onerel->rd_nblocks, numrows, *totalrows)));
!
! return numrows;
}
/*
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql-server/src/backend/commands/vacuum.c,v
retrieving revision 1.269
diff -c -r1.269 vacuum.c
*** src/backend/commands/vacuum.c 29 Nov 2003 19:51:47 -0000 1.269
--- src/backend/commands/vacuum.c 1 Jan 2004 22:32:01 -0000
***************
*** 108,114 ****
/* non-export function prototypes */
! static List *getrels(const RangeVar *vacrel, const char *stmttype);
static void vac_update_dbstats(Oid dbid,
TransactionId vacuumXID,
TransactionId frozenXID);
--- 108,114 ----
/* non-export function prototypes */
! static List *get_rel_oids(const RangeVar *vacrel, const char *stmttype);
static void vac_update_dbstats(Oid dbid,
TransactionId vacuumXID,
TransactionId frozenXID);
***************
*** 161,167 ****
TransactionId initialOldestXmin = InvalidTransactionId;
TransactionId initialFreezeLimit = InvalidTransactionId;
bool all_rels;
! List *vrl,
*cur;
if (vacstmt->verbose)
--- 161,167 ----
TransactionId initialOldestXmin = InvalidTransactionId;
TransactionId initialFreezeLimit = InvalidTransactionId;
bool all_rels;
! List *relations,
*cur;
if (vacstmt->verbose)
***************
*** 216,222 ****
all_rels = (vacstmt->relation == NULL);
/* Build list of relations to process (note this lives in vac_context) */
! vrl = getrels(vacstmt->relation, stmttype);
/*
* Formerly, there was code here to prevent more than one VACUUM from
--- 216,222 ----
all_rels = (vacstmt->relation == NULL);
/* Build list of relations to process (note this lives in vac_context) */
! relations = get_rel_oids(vacstmt->relation, stmttype);
/*
* Formerly, there was code here to prevent more than one VACUUM from
***************
*** 282,288 ****
/*
* Loop to process each selected relation.
*/
! foreach(cur, vrl)
{
Oid relid = lfirsto(cur);
--- 282,288 ----
/*
* Loop to process each selected relation.
*/
! foreach(cur, relations)
{
Oid relid = lfirsto(cur);
***************
*** 383,403 ****
* per-relation transactions.
*/
static List *
! getrels(const RangeVar *vacrel, const char *stmttype)
{
! List *vrl = NIL;
MemoryContext oldcontext;
if (vacrel)
{
! /* Process specific relation */
Oid relid;
relid = RangeVarGetRelid(vacrel, false);
/* Make a relation list entry for this guy */
oldcontext = MemoryContextSwitchTo(vac_context);
! vrl = lappendo(vrl, relid);
MemoryContextSwitchTo(oldcontext);
}
else
--- 383,403 ----
* per-relation transactions.
*/
static List *
! get_rel_oids(const RangeVar *vacrel, const char *stmttype)
{
! List *oid_list = NIL;
MemoryContext oldcontext;
if (vacrel)
{
! /* Process a specific relation */
Oid relid;
relid = RangeVarGetRelid(vacrel, false);
/* Make a relation list entry for this guy */
oldcontext = MemoryContextSwitchTo(vac_context);
! oid_list = lappendo(oid_list, relid);
MemoryContextSwitchTo(oldcontext);
}
else
***************
*** 421,427 ****
{
/* Make a relation list entry for this guy */
oldcontext = MemoryContextSwitchTo(vac_context);
! vrl = lappendo(vrl, HeapTupleGetOid(tuple));
MemoryContextSwitchTo(oldcontext);
}
--- 421,427 ----
{
/* Make a relation list entry for this guy */
oldcontext = MemoryContextSwitchTo(vac_context);
! oid_list = lappendo(oid_list, HeapTupleGetOid(tuple));
MemoryContextSwitchTo(oldcontext);
}
***************
*** 429,435 ****
heap_close(pgclass, AccessShareLock);
}
! return vrl;
}
/*
--- 429,435 ----
heap_close(pgclass, AccessShareLock);
}
! return oid_list;
}
/*
***************
*** 818,825 ****
}
/*
! * Check that it's a plain table; we used to do this in getrels() but
! * seems safer to check after we've locked the relation.
*/
if (onerel->rd_rel->relkind != expected_relkind)
{
--- 818,826 ----
}
/*
! * Check that it's a plain table; we used to do this in
! * get_rel_oids() but seems safer to check after we've locked the
! * relation.
*/
if (onerel->rd_rel->relkind != expected_relkind)
{
Index: src/backend/storage/smgr/md.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql-server/src/backend/storage/smgr/md.c,v
retrieving revision 1.99
diff -c -r1.99 md.c
*** src/backend/storage/smgr/md.c 29 Nov 2003 19:51:57 -0000 1.99
--- src/backend/storage/smgr/md.c 1 Jan 2004 22:33:30 -0000
***************
*** 25,42 ****
#include "utils/inval.h"
#include "utils/memutils.h"
-
- #undef DIAGNOSTIC
-
/*
! * The magnetic disk storage manager keeps track of open file descriptors
! * in its own descriptor pool. This happens for two reasons. First, at
! * transaction boundaries, we walk the list of descriptors and flush
! * anything that we've dirtied in the current transaction. Second, we want
! * to support relations larger than the OS' file size limit (often 2GBytes).
! * In order to do that, we break relations up into chunks of < 2GBytes
! * and store one chunk in each of several files that represent the relation.
! * See the BLCKSZ and RELSEG_SIZE configuration constants in include/pg_config.h.
*
* The file descriptor stored in the relation cache (see RelationGetFile())
* is actually an index into the Md_fdvec array. -1 indicates not open.
--- 25,39 ----
#include "utils/inval.h"
#include "utils/memutils.h"
/*
! * The magnetic disk storage manager keeps track of open file
! * descriptors in its own descriptor pool. This is done to make it
! * easier to support relations that are larger than the operating
! * system's file size limit (often 2GBytes). In order to do that, we
! * we break relations up into chunks of < 2GBytes and store one chunk
! * in each of several files that represent the relation. See the
! * BLCKSZ and RELSEG_SIZE configuration constants in
! * include/pg_config.h.
*
* The file descriptor stored in the relation cache (see RelationGetFile())
* is actually an index into the Md_fdvec array. -1 indicates not open.
***************
*** 67,73 ****
static int Md_Free = -1; /* head of freelist of unused fdvec
* entries */
static int CurFd = 0; /* first never-used fdvec index */
! static MemoryContext MdCxt; /* context for all my allocations */
/* routines declared here */
static void mdclose_fd(int fd);
--- 64,70 ----
static int Md_Free = -1; /* head of freelist of unused fdvec
* entries */
static int CurFd = 0; /* first never-used fdvec index */
! static MemoryContext MdCxt; /* context for all md.c allocations */
/* routines declared here */
static void mdclose_fd(int fd);
***************
*** 84,94 ****
/*
* mdinit() -- Initialize private state for magnetic disk storage manager.
*
! * We keep a private table of all file descriptors. Whenever we do
! * a write to one, we mark it dirty in our table. Whenever we force
! * changes to disk, we mark the file descriptor clean. At transaction
! * commit, we force changes to disk for all dirty file descriptors.
! * This routine allocates and initializes the table.
*
* Returns SM_SUCCESS or SM_FAIL with errno set as appropriate.
*/
--- 81,88 ----
/*
* mdinit() -- Initialize private state for magnetic disk storage manager.
*
! * We keep a private table of all file descriptors. This routine
! * allocates and initializes the table.
*
* Returns SM_SUCCESS or SM_FAIL with errno set as appropriate.
*/
***************
*** 247,262 ****
#ifndef LET_OS_MANAGE_FILESIZE
seekpos = (long) (BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)));
! #ifdef DIAGNOSTIC
! if (seekpos >= BLCKSZ * RELSEG_SIZE)
! elog(FATAL, "seekpos too big");
! #endif
#else
seekpos = (long) (BLCKSZ * (blocknum));
#endif
/*
! * Note: because caller obtained blocknum by calling mdnblocks, which
* did a seek(SEEK_END), this seek is often redundant and will be
* optimized away by fd.c. It's not redundant, however, if there is a
* partial page at the end of the file. In that case we want to try
--- 241,253 ----
#ifndef LET_OS_MANAGE_FILESIZE
seekpos = (long) (BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)));
! Assert(seekpos < BLCKSZ * RELSEG_SIZE);
#else
seekpos = (long) (BLCKSZ * (blocknum));
#endif
/*
! * Note: because caller obtained blocknum by calling _mdnblocks, which
* did a seek(SEEK_END), this seek is often redundant and will be
* optimized away by fd.c. It's not redundant, however, if there is a
* partial page at the end of the file. In that case we want to try
***************
*** 282,291 ****
}
#ifndef LET_OS_MANAGE_FILESIZE
! #ifdef DIAGNOSTIC
! if (_mdnblocks(v->mdfd_vfd, BLCKSZ) > ((BlockNumber) RELSEG_SIZE))
! elog(FATAL, "segment too big");
! #endif
#endif
return SM_SUCCESS;
--- 273,279 ----
}
#ifndef LET_OS_MANAGE_FILESIZE
! Assert(_mdnblocks(v->mdfd_vfd, BLCKSZ) <= ((BlockNumber) RELSEG_SIZE));
#endif
return SM_SUCCESS;
***************
*** 335,345 ****
Md_fdvec[vfd].mdfd_flags = (uint16) 0;
#ifndef LET_OS_MANAGE_FILESIZE
Md_fdvec[vfd].mdfd_chain = (MdfdVec *) NULL;
!
! #ifdef DIAGNOSTIC
! if (_mdnblocks(fd, BLCKSZ) > ((BlockNumber) RELSEG_SIZE))
! elog(FATAL, "segment too big");
! #endif
#endif
return vfd;
--- 323,329 ----
Md_fdvec[vfd].mdfd_flags = (uint16) 0;
#ifndef LET_OS_MANAGE_FILESIZE
Md_fdvec[vfd].mdfd_chain = (MdfdVec *) NULL;
! Assert(_mdnblocks(fd, BLCKSZ) <= ((BlockNumber) RELSEG_SIZE));
#endif
return vfd;
***************
*** 348,354 ****
/*
* mdclose() -- Close the specified relation, if it isn't closed already.
*
! * AND FREE fd vector! It may be re-used for other relation!
* reln should be flushed from cache after closing !..
*
* Returns SM_SUCCESS or SM_FAIL with errno set as appropriate.
--- 332,338 ----
/*
* mdclose() -- Close the specified relation, if it isn't closed already.
*
! * AND FREE fd vector! It may be re-used for other relations!
* reln should be flushed from cache after closing !..
*
* Returns SM_SUCCESS or SM_FAIL with errno set as appropriate.
***************
*** 418,428 ****
#ifndef LET_OS_MANAGE_FILESIZE
seekpos = (long) (BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)));
!
! #ifdef DIAGNOSTIC
! if (seekpos >= BLCKSZ * RELSEG_SIZE)
! elog(FATAL, "seekpos too big");
! #endif
#else
seekpos = (long) (BLCKSZ * (blocknum));
#endif
--- 402,408 ----
#ifndef LET_OS_MANAGE_FILESIZE
seekpos = (long) (BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)));
! Assert(seekpos < BLCKSZ * RELSEG_SIZE);
#else
seekpos = (long) (BLCKSZ * (blocknum));
#endif
***************
*** 466,475 ****
#ifndef LET_OS_MANAGE_FILESIZE
seekpos = (long) (BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)));
! #ifdef DIAGNOSTIC
! if (seekpos >= BLCKSZ * RELSEG_SIZE)
! elog(FATAL, "seekpos too big");
! #endif
#else
seekpos = (long) (BLCKSZ * (blocknum));
#endif
--- 446,452 ----
#ifndef LET_OS_MANAGE_FILESIZE
seekpos = (long) (BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)));
! Assert(seekpos < BLCKSZ * RELSEG_SIZE);
#else
seekpos = (long) (BLCKSZ * (blocknum));
#endif
***************
*** 505,514 ****
#ifndef LET_OS_MANAGE_FILESIZE
seekpos = (long) (BLCKSZ * (blkno % ((BlockNumber) RELSEG_SIZE)));
! #ifdef DIAGNOSTIC
! if (seekpos >= BLCKSZ * RELSEG_SIZE)
! elog(FATAL, "seekpos too big");
! #endif
#else
seekpos = (long) (BLCKSZ * (blkno));
#endif
--- 482,488 ----
#ifndef LET_OS_MANAGE_FILESIZE
seekpos = (long) (BLCKSZ * (blkno % ((BlockNumber) RELSEG_SIZE)));
! Assert(seekpos < BLCKSZ * RELSEG_SIZE);
#else
seekpos = (long) (BLCKSZ * (blkno));
#endif
***************
*** 722,729 ****
/*
* mdabort() -- Abort a transaction.
- *
- * Changes need not be forced to disk at transaction abort.
*/
int
mdabort(void)
--- 696,701 ----
***************
*** 748,754 ****
}
/*
! * _fdvec_alloc () -- grab a free (or new) md file descriptor vector.
*/
static int
_fdvec_alloc(void)
--- 720,726 ----
}
/*
! * _fdvec_alloc() -- Grab a free (or new) md file descriptor vector.
*/
static int
_fdvec_alloc(void)
***************
*** 802,808 ****
}
/*
! * _fdvec_free () -- free md file descriptor vector.
*
*/
static
--- 774,780 ----
}
/*
! * _fdvec_free() -- free md file descriptor vector.
*
*/
static
***************
*** 853,871 ****
v->mdfd_flags = (uint16) 0;
#ifndef LET_OS_MANAGE_FILESIZE
v->mdfd_chain = (MdfdVec *) NULL;
!
! #ifdef DIAGNOSTIC
! if (_mdnblocks(fd, BLCKSZ) > ((BlockNumber) RELSEG_SIZE))
! elog(FATAL, "segment too big");
! #endif
#endif
/* all done */
return v;
}
! /* Get the fd for the relation, opening it if it's not already open */
!
static int
_mdfd_getrelnfd(Relation reln)
{
--- 825,842 ----
v->mdfd_flags = (uint16) 0;
#ifndef LET_OS_MANAGE_FILESIZE
v->mdfd_chain = (MdfdVec *) NULL;
! Assert(_mdnblocks(fd, BLCKSZ) >= ((BlockNumber) RELSEG_SIZE));
#endif
/* all done */
return v;
}
! /*
! * _mdfd_getrelnfd() -- Get the (virtual) fd for the relation,
! * opening it if it's not already open
! *
! */
static int
_mdfd_getrelnfd(Relation reln)
{
***************
*** 882,889 ****
return fd;
}
! /* Find the segment of the relation holding the specified block */
!
static MdfdVec *
_mdfd_getseg(Relation reln, BlockNumber blkno)
{
--- 853,863 ----
return fd;
}
! /*
! * _mdfd_getseg() -- Find the segment of the relation holding the
! * specified block
! *
! */
static MdfdVec *
_mdfd_getseg(Relation reln, BlockNumber blkno)
{
***************
*** 942,948 ****
*
* The return value is the kernel descriptor, or -1 on failure.
*/
-
static int
_mdfd_blind_getseg(RelFileNode rnode, BlockNumber blkno)
{
--- 916,921 ----
Index: src/backend/storage/smgr/smgr.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql-server/src/backend/storage/smgr/smgr.c,v
retrieving revision 1.67
diff -c -r1.67 smgr.c
*** src/backend/storage/smgr/smgr.c 12 Dec 2003 18:45:09 -0000 1.67
--- src/backend/storage/smgr/smgr.c 1 Jan 2004 22:23:04 -0000
***************
*** 382,388 ****
}
/*
! * smgrnblocks() -- Calculate the number of POSTGRES blocks in the
* supplied relation.
*
* Returns the number of blocks on success, aborts the current
--- 382,388 ----
}
/*
! * smgrnblocks() -- Calculate the number of blocks in the
* supplied relation.
*
* Returns the number of blocks on success, aborts the current
***************
*** 411,418 ****
}
/*
! * smgrtruncate() -- Truncate supplied relation to a specified number
! * of blocks
*
* Returns the number of blocks on success, aborts the current
* transaction on failure.
--- 411,418 ----
}
/*
! * smgrtruncate() -- Truncate supplied relation to the specified number
! * of blocks
*
* Returns the number of blocks on success, aborts the current
* transaction on failure.
***************
*** 444,450 ****
}
/*
! * smgrDoPendingDeletes() -- take care of relation deletes at end of xact.
*/
int
smgrDoPendingDeletes(bool isCommit)
--- 444,450 ----
}
/*
! * smgrDoPendingDeletes() -- Take care of relation deletes at end of xact.
*/
int
smgrDoPendingDeletes(bool isCommit)
***************
*** 494,500 ****
* smgrcommit() -- Prepare to commit changes made during the current
* transaction.
*
! * This is called before we actually commit.
*/
int
smgrcommit(void)
--- 494,500 ----
* smgrcommit() -- Prepare to commit changes made during the current
* transaction.
*
! * This is called before we actually commit.
*/
int
smgrcommit(void)
***************
*** 538,544 ****
}
/*
! * Sync files to disk at checkpoint time.
*/
int
smgrsync(void)
--- 538,544 ----
}
/*
! * smgrsync() -- Sync files to disk at checkpoint time.
*/
int
smgrsync(void)
Index: src/include/utils/rel.h
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql-server/src/include/utils/rel.h,v
retrieving revision 1.71
diff -c -r1.71 rel.h
*** src/include/utils/rel.h 29 Nov 2003 22:41:16 -0000 1.71
--- src/include/utils/rel.h 1 Jan 2004 22:23:04 -0000
***************
*** 104,110 ****
typedef struct RelationData
{
! File rd_fd; /* open file descriptor, or -1 if none */
RelFileNode rd_node; /* file node (physical identifier) */
BlockNumber rd_nblocks; /* number of blocks in rel */
BlockNumber rd_targblock; /* current insertion target block, or
--- 104,112 ----
typedef struct RelationData
{
! File rd_fd; /* open file descriptor, or -1 if
! * none; this is NOT an operating
! * system file descriptor */
RelFileNode rd_node; /* file node (physical identifier) */
BlockNumber rd_nblocks; /* number of blocks in rel */
BlockNumber rd_targblock; /* current insertion target block, or
***************
*** 220,241 ****
/*
* RelationGetRelid
! *
! * returns the OID of the relation
*/
#define RelationGetRelid(relation) ((relation)->rd_id)
/*
* RelationGetFile
! *
! * Returns the open file descriptor for the rel
*/
#define RelationGetFile(relation) ((relation)->rd_fd)
/*
* RelationGetNumberOfAttributes
! *
! * Returns the number of attributes.
*/
#define RelationGetNumberOfAttributes(relation) ((relation)->rd_rel->relnatts)
--- 222,242 ----
/*
* RelationGetRelid
! * Returns the OID of the relation
*/
#define RelationGetRelid(relation) ((relation)->rd_id)
/*
* RelationGetFile
! * Returns the open file descriptor for the rel, or -1 if
! * none. This is NOT an operating system file descriptor; see md.c
! * for more information
*/
#define RelationGetFile(relation) ((relation)->rd_fd)
/*
* RelationGetNumberOfAttributes
! * Returns the number of attributes in a relation.
*/
#define RelationGetNumberOfAttributes(relation) ((relation)->rd_rel->relnatts)
***************
*** 247,254 ****
/*
* RelationGetRelationName
! *
! * Returns the rel's name.
*
* Note that the name is only unique within the containing namespace.
*/
--- 248,254 ----
/*
* RelationGetRelationName
! * Returns the rel's name.
*
* Note that the name is only unique within the containing namespace.
*/
***************
*** 257,264 ****
/*
* RelationGetNamespace
! *
! * Returns the rel's namespace OID.
*/
#define RelationGetNamespace(relation) \
((relation)->rd_rel->relnamespace)
--- 257,263 ----
/*
* RelationGetNamespace
! * Returns the rel's namespace OID.
*/
#define RelationGetNamespace(relation) \
((relation)->rd_rel->relnamespace)
---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
