Re: [PATCH 4/5] Add pg_xlogdump contrib module

Поиск
Список
Период
Сортировка
От Andres Freund
Тема Re: [PATCH 4/5] Add pg_xlogdump contrib module
Дата
Msg-id 20130204162929.GA22226@awork2.anarazel.de
обсуждение исходный текст
Ответ на Re: [PATCH 4/5] Add pg_xlogdump contrib module  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Ответы Re: [PATCH 4/5] Add pg_xlogdump contrib module  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers
On 2013-02-04 13:22:26 -0300, Alvaro Herrera wrote:
> 
> I didn't like this bit too much:
> 
> > diff --git a/contrib/pg_xlogdump/tables.c b/contrib/pg_xlogdump/tables.c
> > new file mode 100644
> > index 0000000..e947e0d
> > --- /dev/null
> > +++ b/contrib/pg_xlogdump/tables.c
> > @@ -0,0 +1,78 @@
> 
> > +/*
> > + * RmgrTable linked only to functions available outside of the backend.
> > + *
> > + * needs to be synced with src/backend/access/transam/rmgr.c
> > + */
> > +const RmgrData RmgrTable[RM_MAX_ID + 1] = {
> > +    {"XLOG", NULL, xlog_desc, NULL, NULL, NULL},
> > +    {"Transaction", NULL, xact_desc, NULL, NULL, NULL},
> 
> So I propose the following patch instead.  This lets pg_xlogdump compile
> only the file it needs, and not concern itself with the rest of rmgr.c.

Hm. Not sure whats the advantage of that, duplicates about as much and
makes it harder to copy & paste between both files.
I am fine with it, I just don't see a clear advantage of either way.

> diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
> index ce9957e..b751882 100644
> --- a/src/include/access/xlog_internal.h
> +++ b/src/include/access/xlog_internal.h
> @@ -231,15 +231,17 @@ typedef struct xl_end_of_recovery
>  struct XLogRecord;
>  
>  /*
> - * Method table for resource managers.
> + * Method tables for resource managers.
>   *
> - * RmgrTable[] is indexed by RmgrId values (see rmgr.h).
> + * 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
>  {
>      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);
> @@ -247,6 +249,14 @@ typedef struct RmgrData
>  
>  extern const RmgrData RmgrTable[];
>  
> +typedef struct RmgrDescData
> +{
> +    void        (*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
> +} RmgrDescData;
> +
> +extern const RmgrDescData RmgrDescTable[];
> +

I think we would at least need to include rm_name here as well.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Alvaro Herrera
Дата:
Сообщение: Re: [PATCH 4/5] Add pg_xlogdump contrib module
Следующее
От: Simon Riggs
Дата:
Сообщение: Re: GetOldestXmin going backwards is dangerous after all