split rm_name and rm_desc out of rmgr.c
Hi,
pg_xlogdump needs access to the *_desc functions for each rmgr. We
already moved forward quite a bit by splitting those functions out of
their containing files; so now they are compilable separately. Good.
The remaining task is enabling the code to find those functions in the
first place; currently, the function pointers live in rmgr.c which is
not compilable by frontend code because it contains pointers to other
functions. Hence the attached patch splits RmgrData into two; the names
and rm_desc functions go into a new file which can be compiled easily by
frontend.
Proposed patch attached.
This comes from
/messages/by-id/20130204180327.GH4963@alvh.no-ip.org
which is part of the pg_xlogdump patch in commitfest.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
split-rm_desc-2.patchtext/x-diff; charset=us-asciiDownload
*** a/src/backend/access/transam/Makefile
--- b/src/backend/access/transam/Makefile
***************
*** 12,20 **** subdir = src/backend/access/transam
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
! OBJS = clog.o transam.o varsup.o xact.o rmgr.o slru.o subtrans.o multixact.o \
! timeline.o twophase.o twophase_rmgr.o xlog.o xlogarchive.o xlogfuncs.o \
! xlogreader.o xlogutils.o
include $(top_srcdir)/src/backend/common.mk
--- 12,20 ----
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
! OBJS = clog.o multixact.o rmgrdesc.o rmgr.o slru.o subtrans.o timeline.o \
! transam.o twophase.o twophase_rmgr.o varsup.o xact.o xlogarchive.o \
! xlogfuncs.o xlog.o xlogreader.o xlogutils.o
include $(top_srcdir)/src/backend/common.mk
*** a/src/backend/access/transam/rmgr.c
--- b/src/backend/access/transam/rmgr.c
***************
*** 24,46 ****
#include "storage/standby.h"
#include "utils/relmapper.h"
const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! {"XLOG", xlog_redo, xlog_desc, NULL, NULL, NULL},
! {"Transaction", xact_redo, xact_desc, NULL, NULL, NULL},
! {"Storage", smgr_redo, smgr_desc, NULL, NULL, NULL},
! {"CLOG", clog_redo, clog_desc, NULL, NULL, NULL},
! {"Database", dbase_redo, dbase_desc, NULL, NULL, NULL},
! {"Tablespace", tblspc_redo, tblspc_desc, NULL, NULL, NULL},
! {"MultiXact", multixact_redo, multixact_desc, NULL, NULL, NULL},
! {"RelMap", relmap_redo, relmap_desc, NULL, NULL, NULL},
! {"Standby", standby_redo, standby_desc, NULL, NULL, NULL},
! {"Heap2", heap2_redo, heap2_desc, NULL, NULL, NULL},
! {"Heap", heap_redo, heap_desc, NULL, NULL, NULL},
! {"Btree", btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
! {"Hash", hash_redo, hash_desc, NULL, NULL, NULL},
! {"Gin", gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
! {"Gist", gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL},
! {"Sequence", seq_redo, seq_desc, NULL, NULL, NULL},
! {"SPGist", spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL}
};
--- 24,47 ----
#include "storage/standby.h"
#include "utils/relmapper.h"
+ /* See rmgrdesc.c, too */
const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! {xlog_redo, NULL, NULL, NULL},
! {xact_redo, NULL, NULL, NULL},
! {smgr_redo, NULL, NULL, NULL},
! {clog_redo, NULL, NULL, NULL},
! {dbase_redo, NULL, NULL, NULL},
! {tblspc_redo, NULL, NULL, NULL},
! {multixact_redo, NULL, NULL, NULL},
! {relmap_redo, NULL, NULL, NULL},
! {standby_redo, NULL, NULL, NULL},
! {heap2_redo, NULL, NULL, NULL},
! {heap_redo, NULL, NULL, NULL},
! {btree_redo, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
! {hash_redo, NULL, NULL, NULL},
! {gin_redo, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
! {gist_redo, gist_xlog_startup, gist_xlog_cleanup, NULL},
! {seq_redo, NULL, NULL, NULL},
! {spg_redo, spg_xlog_startup, spg_xlog_cleanup, NULL}
};
*** /dev/null
--- b/src/backend/access/transam/rmgrdesc.c
***************
*** 0 ****
--- 1,47 ----
+ /*
+ * rmgrdesc.c
+ *
+ * Resource managers descriptor function definitions
+ *
+ * src/backend/access/transam/rmgrdesc.c
+ */
+ #include "postgres.h"
+
+ #include "access/clog.h"
+ #include "access/gin.h"
+ #include "access/gist_private.h"
+ #include "access/hash.h"
+ #include "access/heapam_xlog.h"
+ #include "access/multixact.h"
+ #include "access/nbtree.h"
+ #include "access/spgist.h"
+ #include "access/xact.h"
+ #include "access/xlog_internal.h"
+ #include "catalog/storage_xlog.h"
+ #include "commands/dbcommands.h"
+ #include "commands/sequence.h"
+ #include "commands/tablespace.h"
+ #include "storage/standby.h"
+ #include "utils/relmapper.h"
+
+ /* See rmgr.c, too */
+
+ const RmgrDescData RmgrDescTable[RM_MAX_ID + 1] = {
+ {"XLOG", xlog_desc},
+ {"Transaction", xact_desc},
+ {"Storage", smgr_desc},
+ {"CLOG", clog_desc},
+ {"Database", dbase_desc},
+ {"Tablespace", tblspc_desc},
+ {"MultiXact", multixact_desc},
+ {"RelMap", relmap_desc},
+ {"Standby", standby_desc},
+ {"Heap2", heap2_desc},
+ {"Heap", heap_desc},
+ {"Btree", btree_desc},
+ {"Hash", hash_desc},
+ {"Gin", gin_desc},
+ {"Gist", gist_desc},
+ {"Sequence", seq_desc},
+ {"SPGist", spg_desc},
+ };
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 1042,1048 **** begin:;
if (rdata->data != NULL)
{
appendStringInfo(&buf, " - ");
! RmgrTable[rechdr->xl_rmid].rm_desc(&buf, rechdr->xl_info, rdata->data);
}
elog(LOG, "%s", buf.data);
pfree(buf.data);
--- 1042,1048 ----
if (rdata->data != NULL)
{
appendStringInfo(&buf, " - ");
! RmgrDescTable[rechdr->xl_rmid].rm_desc(&buf, rechdr->xl_info, rdata->data);
}
elog(LOG, "%s", buf.data);
pfree(buf.data);
***************
*** 5352,5358 **** StartupXLOG(void)
(uint32) (EndRecPtr >> 32), (uint32) EndRecPtr);
xlog_outrec(&buf, record);
appendStringInfo(&buf, " - ");
! RmgrTable[record->xl_rmid].rm_desc(&buf,
record->xl_info,
XLogRecGetData(record));
elog(LOG, "%s", buf.data);
--- 5352,5358 ----
(uint32) (EndRecPtr >> 32), (uint32) EndRecPtr);
xlog_outrec(&buf, record);
appendStringInfo(&buf, " - ");
! RmgrDescTable[record->xl_rmid].rm_desc(&buf,
record->xl_info,
XLogRecGetData(record));
elog(LOG, "%s", buf.data);
***************
*** 7882,7888 **** xlog_outrec(StringInfo buf, XLogRecord *record)
appendStringInfo(buf, "; bkpb%d", i);
}
! appendStringInfo(buf, ": %s", RmgrTable[record->xl_rmid].rm_name);
}
#endif /* WAL_DEBUG */
--- 7882,7888 ----
appendStringInfo(buf, "; bkpb%d", i);
}
! appendStringInfo(buf, ": %s", RmgrDescTable[record->xl_rmid].rm_name);
}
#endif /* WAL_DEBUG */
***************
*** 8924,8930 **** rm_redo_error_callback(void *arg)
StringInfoData buf;
initStringInfo(&buf);
! RmgrTable[record->xl_rmid].rm_desc(&buf,
record->xl_info,
XLogRecGetData(record));
--- 8924,8930 ----
StringInfoData buf;
initStringInfo(&buf);
! RmgrDescTable[record->xl_rmid].rm_desc(&buf,
record->xl_info,
XLogRecGetData(record));
*** a/src/include/access/rmgr.h
--- b/src/include/access/rmgr.h
***************
*** 14,20 **** typedef uint8 RmgrId;
* Built-in resource managers
*
* Note: RM_MAX_ID could be as much as 255 without breaking the XLOG file
! * format, but we keep it small to minimize the size of RmgrTable[].
*/
#define RM_XLOG_ID 0
#define RM_XACT_ID 1
--- 14,21 ----
* Built-in resource managers
*
* Note: RM_MAX_ID could be as much as 255 without breaking the XLOG file
! * format, but we keep it small to minimize the size of RmgrTable[]/
! * RmgrDescTable[].
*/
#define RM_XLOG_ID 0
#define RM_XACT_ID 1
*** a/src/include/access/xlog_internal.h
--- b/src/include/access/xlog_internal.h
***************
*** 231,245 **** typedef struct xl_end_of_recovery
struct XLogRecord;
/*
! * Method table for resource managers.
*
! * RmgrTable[] is indexed by RmgrId values (see rmgr.h).
*/
typedef struct RmgrData
{
- const char *rm_name;
void (*rm_redo) (XLogRecPtr lsn, struct XLogRecord *rptr);
- void (*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
void (*rm_startup) (void);
void (*rm_cleanup) (void);
bool (*rm_safe_restartpoint) (void);
--- 231,246 ----
struct XLogRecord;
/*
! * Method tables for resource managers.
*
! * RmgrDescData (for textual descriptor functions) is split so that the file it
! * lives in can be used by frontend programs.
! *
! * RmgrTable[] and RmgrDescTable[] are indexed by RmgrId values (see rmgr.h).
*/
typedef struct RmgrData
{
void (*rm_redo) (XLogRecPtr lsn, struct XLogRecord *rptr);
void (*rm_startup) (void);
void (*rm_cleanup) (void);
bool (*rm_safe_restartpoint) (void);
***************
*** 247,252 **** typedef struct RmgrData
--- 248,262 ----
extern const RmgrData RmgrTable[];
+ typedef struct RmgrDescData
+ {
+ const char *rm_name;
+ void (*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
+ } RmgrDescData;
+
+ extern const RmgrDescData RmgrDescTable[];
+
+
/*
* Exported to support xlog switching from checkpointer
*/
On 4 February 2013 20:57, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
pg_xlogdump needs access to the *_desc functions for each rmgr. We
already moved forward quite a bit by splitting those functions out of
their containing files; so now they are compilable separately. Good.
The remaining task is enabling the code to find those functions in the
first place; currently, the function pointers live in rmgr.c which is
not compilable by frontend code because it contains pointers to other
functions. Hence the attached patch splits RmgrData into two; the names
and rm_desc functions go into a new file which can be compiled easily by
frontend.Proposed patch attached.
Not meaning to cause you extra work, but some kind of id as the first
field of each structure would allow a cross-check that there is no
misconfiguration.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Simon Riggs wrote:
On 4 February 2013 20:57, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
pg_xlogdump needs access to the *_desc functions for each rmgr. We
already moved forward quite a bit by splitting those functions out of
their containing files; so now they are compilable separately. Good.
The remaining task is enabling the code to find those functions in the
first place; currently, the function pointers live in rmgr.c which is
not compilable by frontend code because it contains pointers to other
functions. Hence the attached patch splits RmgrData into two; the names
and rm_desc functions go into a new file which can be compiled easily by
frontend.Proposed patch attached.
Not meaning to cause you extra work, but some kind of id as the first
field of each structure would allow a cross-check that there is no
misconfiguration.
Well, we could add the rmgr_id as the first struct member to both
tables and add an Assert() somewhere in xlog.c. However, do we really
need that? These tables are almost immutable, and failure to edit both
simultaneously should be promptly detected.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
pg_xlogdump needs access to the *_desc functions for each rmgr. We
already moved forward quite a bit by splitting those functions out of
their containing files; so now they are compilable separately. Good.
The remaining task is enabling the code to find those functions in the
first place; currently, the function pointers live in rmgr.c which is
not compilable by frontend code because it contains pointers to other
functions. Hence the attached patch splits RmgrData into two; the names
and rm_desc functions go into a new file which can be compiled easily by
frontend.
Proposed patch attached.
This seems pretty ugly to me.
Couldn't we do something similar to the design for SQL keyword constants,
wherein the actual data is in macros in a header file (providing exactly
one source of truth for each RM) and then various .c files can #include
that after #defining the macro as they need? See
src/include/parser/kwlist.h and the files that include that.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-02-04 17:27:39 -0500, Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
pg_xlogdump needs access to the *_desc functions for each rmgr. We
already moved forward quite a bit by splitting those functions out of
their containing files; so now they are compilable separately. Good.
The remaining task is enabling the code to find those functions in the
first place; currently, the function pointers live in rmgr.c which is
not compilable by frontend code because it contains pointers to other
functions. Hence the attached patch splits RmgrData into two; the names
and rm_desc functions go into a new file which can be compiled easily by
frontend.Proposed patch attached.
This seems pretty ugly to me.
Couldn't we do something similar to the design for SQL keyword constants,
wherein the actual data is in macros in a header file (providing exactly
one source of truth for each RM) and then various .c files can #include
that after #defining the macro as they need? See
src/include/parser/kwlist.h and the files that include that.
My original patch just surrounded the backend-only functions by a macro
which was defined to NULL in #ifdef FRONTEND. I still think thats the
best way to go. People objected to that though...
Other than that I find duplicating the whole table by far the best
approach. Splitting parts off seems to be more complex than warranted
without actually getting rid of duplication.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
pg_xlogdump needs access to the *_desc functions for each rmgr. We
already moved forward quite a bit by splitting those functions out of
their containing files; so now they are compilable separately. Good.
The remaining task is enabling the code to find those functions in the
first place; currently, the function pointers live in rmgr.c which is
not compilable by frontend code because it contains pointers to other
functions. Hence the attached patch splits RmgrData into two; the names
and rm_desc functions go into a new file which can be compiled easily by
frontend.Proposed patch attached.
This seems pretty ugly to me.
Couldn't we do something similar to the design for SQL keyword constants,
wherein the actual data is in macros in a header file (providing exactly
one source of truth for each RM) and then various .c files can #include
that after #defining the macro as they need? See
src/include/parser/kwlist.h and the files that include that.
Meh. I proposed this months ago and was shot down. I still like it
better than what I propose here, so I will resurrect it.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane wrote:
Couldn't we do something similar to the design for SQL keyword constants,
wherein the actual data is in macros in a header file (providing exactly
one source of truth for each RM) and then various .c files can #include
that after #defining the macro as they need?
Here are two patches implementing this idea. The first one is simpler
and just replaces the table in rmgr.c with an appropriate PG_RMGR
define.
The second one touches rmgr.h as well. That file currently has a list
of #defines with symbolic rmgr names and their numeric IDs. The
approach in the second patch is to turn these into "extern const RmgrId"
instead, and use a second inclusion of rmgrlist.h in rmgr.c that assigns
them the values as consts. This has the disadvantage that the array
size RM_MAX_ID cannot use the symbolic RM_SPGIST_ID value, but instead
it needs a literal "16". Otherwise the compile complains:
error: variably modified ‘RmgrTable’ at file scope
I am not too sure about that second change. However, I'm a bit uneasy
about leaving a second list of rmgrs around; since we're creating what
should be the canonical list of rmgrs, it makes sense to reduce the two
copies we have. Better ideas for that second list are welcome.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
split-rm_desc-3.patchtext/x-diff; charset=us-asciiDownload
*** a/src/backend/access/transam/rmgr.c
--- b/src/backend/access/transam/rmgr.c
***************
*** 24,46 ****
#include "storage/standby.h"
#include "utils/relmapper.h"
const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! {"XLOG", xlog_redo, xlog_desc, NULL, NULL, NULL},
! {"Transaction", xact_redo, xact_desc, NULL, NULL, NULL},
! {"Storage", smgr_redo, smgr_desc, NULL, NULL, NULL},
! {"CLOG", clog_redo, clog_desc, NULL, NULL, NULL},
! {"Database", dbase_redo, dbase_desc, NULL, NULL, NULL},
! {"Tablespace", tblspc_redo, tblspc_desc, NULL, NULL, NULL},
! {"MultiXact", multixact_redo, multixact_desc, NULL, NULL, NULL},
! {"RelMap", relmap_redo, relmap_desc, NULL, NULL, NULL},
! {"Standby", standby_redo, standby_desc, NULL, NULL, NULL},
! {"Heap2", heap2_redo, heap2_desc, NULL, NULL, NULL},
! {"Heap", heap_redo, heap_desc, NULL, NULL, NULL},
! {"Btree", btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
! {"Hash", hash_redo, hash_desc, NULL, NULL, NULL},
! {"Gin", gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
! {"Gist", gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL},
! {"Sequence", seq_redo, seq_desc, NULL, NULL, NULL},
! {"SPGist", spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL}
};
--- 24,32 ----
#include "storage/standby.h"
#include "utils/relmapper.h"
+ #define PG_RMGR(name,redo,desc,startup,cleanup,restartpoint) \
+ { name, redo, desc, startup, cleanup, restartpoint },
const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! #include "access/rmgrlist.h"
};
*** /dev/null
--- b/src/include/access/rmgrlist.h
***************
*** 0 ****
--- 1,39 ----
+ /*---------------------------------------------------------------------------
+ * rmgrlist.h
+ *
+ * The resource manager list is kept in its own source file for possible
+ * use by automatic tools. The exact representation of a rmgr is determined
+ * by the PG_RMGR macro, which is not defined in this file; it can be
+ * defined by the caller for special purposes.
+ *
+ * Portions Copyright (c) 1996-2013, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/access/rmgrlist.h
+ *---------------------------------------------------------------------------
+ */
+
+ /* there is deliberately not an #ifndef RMGRLIST_H here */
+
+ /*
+ * List of resource manager entries.
+ */
+
+ /* name, redo, desc, startup, cleanup, restartpoint */
+ PG_RMGR("XLOG", xlog_redo, xlog_desc, NULL, NULL, NULL)
+ PG_RMGR("Transaction", xact_redo, xact_desc, NULL, NULL, NULL)
+ PG_RMGR("Storage", smgr_redo, smgr_desc, NULL, NULL, NULL)
+ PG_RMGR("CLOG", clog_redo, clog_desc, NULL, NULL, NULL)
+ PG_RMGR("Database", dbase_redo, dbase_desc, NULL, NULL, NULL)
+ PG_RMGR("Tablespace", tblspc_redo, tblspc_desc, NULL, NULL, NULL)
+ PG_RMGR("MultiXact", multixact_redo, multixact_desc, NULL, NULL, NULL)
+ PG_RMGR("RelMap", relmap_redo, relmap_desc, NULL, NULL, NULL)
+ PG_RMGR("Standby", standby_redo, standby_desc, NULL, NULL, NULL)
+ PG_RMGR("Heap2", heap2_redo, heap2_desc, NULL, NULL, NULL)
+ PG_RMGR("Heap", heap_redo, heap_desc, NULL, NULL, NULL)
+ PG_RMGR("Btree", btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint)
+ PG_RMGR("Hash", hash_redo, hash_desc, NULL, NULL, NULL)
+ PG_RMGR("Gin", gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint)
+ PG_RMGR("Gist", gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL)
+ PG_RMGR("Sequence", seq_redo, seq_desc, NULL, NULL, NULL)
+ PG_RMGR("SPGist", spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL)
split-rm_desc-3a.patchtext/x-diff; charset=us-asciiDownload
*** a/src/backend/access/transam/rmgr.c
--- b/src/backend/access/transam/rmgr.c
***************
*** 24,46 ****
#include "storage/standby.h"
#include "utils/relmapper.h"
const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! {"XLOG", xlog_redo, xlog_desc, NULL, NULL, NULL},
! {"Transaction", xact_redo, xact_desc, NULL, NULL, NULL},
! {"Storage", smgr_redo, smgr_desc, NULL, NULL, NULL},
! {"CLOG", clog_redo, clog_desc, NULL, NULL, NULL},
! {"Database", dbase_redo, dbase_desc, NULL, NULL, NULL},
! {"Tablespace", tblspc_redo, tblspc_desc, NULL, NULL, NULL},
! {"MultiXact", multixact_redo, multixact_desc, NULL, NULL, NULL},
! {"RelMap", relmap_redo, relmap_desc, NULL, NULL, NULL},
! {"Standby", standby_redo, standby_desc, NULL, NULL, NULL},
! {"Heap2", heap2_redo, heap2_desc, NULL, NULL, NULL},
! {"Heap", heap_redo, heap_desc, NULL, NULL, NULL},
! {"Btree", btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
! {"Hash", hash_redo, hash_desc, NULL, NULL, NULL},
! {"Gin", gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
! {"Gist", gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL},
! {"Sequence", seq_redo, seq_desc, NULL, NULL, NULL},
! {"SPGist", spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL}
};
--- 24,39 ----
#include "storage/standby.h"
#include "utils/relmapper.h"
+ #define PG_RMGR(symname,num,name,redo,desc,startup,cleanup,restartpoint) \
+ { name, redo, desc, startup, cleanup, restartpoint },
const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! #include "access/rmgrlist.h"
};
+
+ #undef PG_RMGR
+
+ #define PG_RMGR(symname,num,name,redo,desc,startup,cleanup,restartpoint) \
+ const RmgrId symname = num;
+ #include "access/rmgrlist.h"
+ #undef PG_RMGR
*** a/src/include/access/rmgr.h
--- b/src/include/access/rmgr.h
***************
*** 16,39 **** typedef uint8 RmgrId;
* Note: RM_MAX_ID could be as much as 255 without breaking the XLOG file
* format, but we keep it small to minimize the size of RmgrTable[].
*/
! #define RM_XLOG_ID 0
! #define RM_XACT_ID 1
! #define RM_SMGR_ID 2
! #define RM_CLOG_ID 3
! #define RM_DBASE_ID 4
! #define RM_TBLSPC_ID 5
! #define RM_MULTIXACT_ID 6
! #define RM_RELMAP_ID 7
! #define RM_STANDBY_ID 8
! #define RM_HEAP2_ID 9
! #define RM_HEAP_ID 10
! #define RM_BTREE_ID 11
! #define RM_HASH_ID 12
! #define RM_GIN_ID 13
! #define RM_GIST_ID 14
! #define RM_SEQ_ID 15
! #define RM_SPGIST_ID 16
! #define RM_MAX_ID RM_SPGIST_ID
#endif /* RMGR_H */
--- 16,28 ----
* Note: RM_MAX_ID could be as much as 255 without breaking the XLOG file
* format, but we keep it small to minimize the size of RmgrTable[].
*/
! #define PG_RMGR(symname,id,name,redo,desc,startup,cleanup,restartpoint) \
! extern const RmgrId symname;
! #include "access/rmgrlist.h"
! #undef PG_RMGR
!
! /* beware! must be kept in sync with rmgrlist.h */
! #define RM_MAX_ID 16
#endif /* RMGR_H */
*** /dev/null
--- b/src/include/access/rmgrlist.h
***************
*** 0 ****
--- 1,42 ----
+ /*---------------------------------------------------------------------------
+ * rmgrlist.h
+ *
+ * The resource manager list is kept in its own source file for possible
+ * use by automatic tools. The exact representation of a rmgr is determined
+ * by the PG_RMGR macro, which is not defined in this file; it can be
+ * defined by the caller for special purposes.
+ *
+ * Portions Copyright (c) 1996-2013, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/access/rmgrlist.h
+ *---------------------------------------------------------------------------
+ */
+
+ /* there is deliberately not an #ifndef RMGRLIST_H here */
+
+ /*
+ * List of resource manager entries.
+ *
+ * Beware! The numerical value of the last entry must be kept in sync with
+ * the RM_MAX_ID in rmgr.h.
+ */
+
+ /* symname, id, name, redo, desc, startup, cleanup, restartpoint */
+ PG_RMGR(RM_XLOG_ID, 0, "XLOG", xlog_redo, xlog_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_XACT_ID, 1, "Transaction", xact_redo, xact_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_SMGR_ID, 2, "Storage", smgr_redo, smgr_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_CLOG_ID, 3, "CLOG", clog_redo, clog_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_DBASE_ID, 4, "Database", dbase_redo, dbase_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_TBLSPC_ID, 5, "Tablespace", tblspc_redo, tblspc_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_MULTIXACT_ID, 6, "MultiXact", multixact_redo, multixact_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_RELMAP_ID, 7, "RelMap", relmap_redo, relmap_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_STANDBY_ID, 8, "Standby", standby_redo, standby_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_HEAP2_ID, 9, "Heap2", heap2_redo, heap2_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_HEAP_ID, 10, "Heap", heap_redo, heap_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_BTREE_ID, 11, "Btree", btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint)
+ PG_RMGR(RM_HASH_ID, 12, "Hash", hash_redo, hash_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_GIN_ID, 13, "Gin", gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint)
+ PG_RMGR(RM_GIST_ID, 14, "Gist", gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL)
+ PG_RMGR(RM_SEQ_ID, 15, "Sequence", seq_redo, seq_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_SPGIST_ID, 16, "SPGist", spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL)
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Tom Lane wrote:
Couldn't we do something similar to the design for SQL keyword constants,
wherein the actual data is in macros in a header file (providing exactly
one source of truth for each RM) and then various .c files can #include
that after #defining the macro as they need?
Here are two patches implementing this idea. The first one is simpler
and just replaces the table in rmgr.c with an appropriate PG_RMGR
define.
The second one touches rmgr.h as well. That file currently has a list
of #defines with symbolic rmgr names and their numeric IDs.
Unifying that with this one-source-of-truth seems attractive ...
The
approach in the second patch is to turn these into "extern const RmgrId"
instead, and use a second inclusion of rmgrlist.h in rmgr.c that assigns
them the values as consts.
... but I don't especially like that implementation, as it will result
in nonzero code bloat and runtime cost due to replacing all those
constants with global-variable references. Couldn't you instead set it
up as an enum definition? Something like
#define PG_RMGR(...) sym = num,
typedef enum {
#include ...
RM_NEXT_ID
} RmgrIds;
#define RM_LAST_ID (RM_NEXT_ID-1)
I'm not actually sure that we need the explicit numbers in the macros
if we do this. That is, we could just have "#define PG_RMGR(...) sym,"
and say that the order of the entries in rmgrlist.h is what defines
the manager IDs. The original coding allowed for gaps in the ID list
but I don't see much value in that.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
The
approach in the second patch is to turn these into "extern const RmgrId"
instead, and use a second inclusion of rmgrlist.h in rmgr.c that assigns
them the values as consts.... but I don't especially like that implementation, as it will result
in nonzero code bloat and runtime cost due to replacing all those
constants with global-variable references. Couldn't you instead set it
up as an enum definition?
That seems to work. I would like to have some way of specifying that
the enum members should be of type RmgrId, but I don't think there's any
way to do that.
Patch attached.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
split-rm_desc-4.patchtext/x-diff; charset=us-asciiDownload
*** a/src/backend/access/transam/rmgr.c
--- b/src/backend/access/transam/rmgr.c
***************
*** 24,46 ****
#include "storage/standby.h"
#include "utils/relmapper.h"
const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! {"XLOG", xlog_redo, xlog_desc, NULL, NULL, NULL},
! {"Transaction", xact_redo, xact_desc, NULL, NULL, NULL},
! {"Storage", smgr_redo, smgr_desc, NULL, NULL, NULL},
! {"CLOG", clog_redo, clog_desc, NULL, NULL, NULL},
! {"Database", dbase_redo, dbase_desc, NULL, NULL, NULL},
! {"Tablespace", tblspc_redo, tblspc_desc, NULL, NULL, NULL},
! {"MultiXact", multixact_redo, multixact_desc, NULL, NULL, NULL},
! {"RelMap", relmap_redo, relmap_desc, NULL, NULL, NULL},
! {"Standby", standby_redo, standby_desc, NULL, NULL, NULL},
! {"Heap2", heap2_redo, heap2_desc, NULL, NULL, NULL},
! {"Heap", heap_redo, heap_desc, NULL, NULL, NULL},
! {"Btree", btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
! {"Hash", hash_redo, hash_desc, NULL, NULL, NULL},
! {"Gin", gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
! {"Gist", gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL},
! {"Sequence", seq_redo, seq_desc, NULL, NULL, NULL},
! {"SPGist", spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL}
};
--- 24,33 ----
#include "storage/standby.h"
#include "utils/relmapper.h"
+ /* must be kept in sync with RmgrData definition in xlog_internal.h */
+ #define PG_RMGR(symname,name,redo,desc,startup,cleanup,restartpoint) \
+ { name, redo, desc, startup, cleanup, restartpoint },
const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! #include "access/rmgrlist.h"
};
*** a/src/include/access/rmgr.h
--- b/src/include/access/rmgr.h
***************
*** 13,39 **** typedef uint8 RmgrId;
/*
* Built-in resource managers
*
* Note: RM_MAX_ID could be as much as 255 without breaking the XLOG file
* format, but we keep it small to minimize the size of RmgrTable[].
*/
! #define RM_XLOG_ID 0
! #define RM_XACT_ID 1
! #define RM_SMGR_ID 2
! #define RM_CLOG_ID 3
! #define RM_DBASE_ID 4
! #define RM_TBLSPC_ID 5
! #define RM_MULTIXACT_ID 6
! #define RM_RELMAP_ID 7
! #define RM_STANDBY_ID 8
! #define RM_HEAP2_ID 9
! #define RM_HEAP_ID 10
! #define RM_BTREE_ID 11
! #define RM_HASH_ID 12
! #define RM_GIN_ID 13
! #define RM_GIST_ID 14
! #define RM_SEQ_ID 15
! #define RM_SPGIST_ID 16
! #define RM_MAX_ID RM_SPGIST_ID
#endif /* RMGR_H */
--- 13,35 ----
/*
* Built-in resource managers
*
+ * The actual numerical values for each rmgr ID are defined by the order
+ * of entries in rmgrlist.h.
+ *
* Note: RM_MAX_ID could be as much as 255 without breaking the XLOG file
* format, but we keep it small to minimize the size of RmgrTable[].
*/
! #define PG_RMGR(symname,name,redo,desc,startup,cleanup,restartpoint) \
! symname,
!
! typedef enum RmgrIds
! {
! #include "access/rmgrlist.h"
! RM_NEXT_ID
! } RmgrIds;
!
! #undef PG_RMGR
! #define RM_MAX_ID (RM_NEXT_ID - 1)
#endif /* RMGR_H */
*** /dev/null
--- b/src/include/access/rmgrlist.h
***************
*** 0 ****
--- 1,42 ----
+ /*---------------------------------------------------------------------------
+ * rmgrlist.h
+ *
+ * The resource manager list is kept in its own source file for possible
+ * use by automatic tools. The exact representation of a rmgr is determined
+ * by the PG_RMGR macro, which is not defined in this file; it can be
+ * defined by the caller for special purposes.
+ *
+ * Portions Copyright (c) 1996-2013, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/access/rmgrlist.h
+ *---------------------------------------------------------------------------
+ */
+
+ /* there is deliberately not an #ifndef RMGRLIST_H here */
+
+ /*
+ * List of resource manager entries. Note that order of entries defines the
+ * numerical values of each rmgr's ID.
+ *
+ * Changes to this list possibly need a XLOG_PAGE_MAGIC bump.
+ */
+
+ /* symname, name, redo, desc, startup, cleanup, restartpoint */
+ PG_RMGR(RM_XLOG_ID, "XLOG", xlog_redo, xlog_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_XACT_ID, "Transaction", xact_redo, xact_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_SMGR_ID, "Storage", smgr_redo, smgr_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_CLOG_ID, "CLOG", clog_redo, clog_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_DBASE_ID, "Database", dbase_redo, dbase_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_TBLSPC_ID, "Tablespace", tblspc_redo, tblspc_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_MULTIXACT_ID, "MultiXact", multixact_redo, multixact_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_RELMAP_ID, "RelMap", relmap_redo, relmap_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_STANDBY_ID, "Standby", standby_redo, standby_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_HEAP2_ID, "Heap2", heap2_redo, heap2_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_HEAP_ID, "Heap", heap_redo, heap_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_BTREE_ID, "Btree", btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint)
+ PG_RMGR(RM_HASH_ID, "Hash", hash_redo, hash_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_GIN_ID, "Gin", gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint)
+ PG_RMGR(RM_GIST_ID, "Gist", gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL)
+ PG_RMGR(RM_SEQ_ID, "Sequence", seq_redo, seq_desc, NULL, NULL, NULL)
+ PG_RMGR(RM_SPGIST_ID, "SPGist", spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL)
*** a/src/include/access/xlog_internal.h
--- b/src/include/access/xlog_internal.h
***************
*** 233,239 **** struct XLogRecord;
/*
* Method table for resource managers.
*
! * RmgrTable[] is indexed by RmgrId values (see rmgr.h).
*/
typedef struct RmgrData
{
--- 233,242 ----
/*
* Method table for resource managers.
*
! * This struct must be kept in sync with the PG_RMGR definition in
! * rmgr.c.
! *
! * RmgrTable[] is indexed by RmgrId values (see rmgrlist.h).
*/
typedef struct RmgrData
{
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Patch attached.
Seems like a couple of the comments could use updates:
* Note: RM_MAX_ID could be as much as 255 without breaking the XLOG file
* format, but we keep it small to minimize the size of RmgrTable[].
This is no longer particularly sensible, since we're no longer making
any provision for wasted RmgrIds. Perhaps rephrase as "RM_MAX_ID must
fit in RmgrId; widening that type will affect the XLOG file format."
+ * List of resource manager entries. Note that order of entries defines the + * numerical values of each rmgr's ID. + * + * Changes to this list possibly need a XLOG_PAGE_MAGIC bump.
Probably also a good idea to state explicitly that new entries should go
at the end to avoid moving the IDs of existing entries.
Works for me otherwise.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/5/13 3:47 PM, Alvaro Herrera wrote:
Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
The
approach in the second patch is to turn these into "extern const RmgrId"
instead, and use a second inclusion of rmgrlist.h in rmgr.c that assigns
them the values as consts.... but I don't especially like that implementation, as it will result
in nonzero code bloat and runtime cost due to replacing all those
constants with global-variable references. Couldn't you instead set it
up as an enum definition?That seems to work. I would like to have some way of specifying that
the enum members should be of type RmgrId, but I don't think there's any
way to do that.Patch attached.
This has broken cpluspluscheck:
./src/include/access/rmgrlist.h:28:8: error: expected constructor, destructor, or type conversion before '(' token
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter_e@gmx.net> writes:
On 2/5/13 3:47 PM, Alvaro Herrera wrote:
Patch attached.
This has broken cpluspluscheck:
./src/include/access/rmgrlist.h:28:8: error: expected constructor, destructor, or type conversion before '(' token
cpluspluscheck has an explicit exclusion for kwlist.h, looks like it
needs one for rmgrlist.h as well, since neither of them are meant to
compile standalone.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers