Lists: | pgsql-hackers |
---|
From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
---|---|
To: | PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
Subject: | rm_desc signature |
Date: | 2014-06-13 11:37:33 |
Message-ID: | 539AE27D.3010003@vmware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
As part of the WAL-format changing patch I've been working on, I changed
the signature of the rm_desc function from:
void (*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
void (*rm_desc) (StringInfo buf, XLogRecord *record);
The WAL-format patch needed that because it added more functions/macros
for working with XLogRecords, also used by rm_desc routines, but it
seems like a sensible change anyway. IMHO it was always a bit strange
that rm_desc was passed the info field and record payload separately.
So I propose to do that change as a separate commit. Per attached. This
has no functional changes, it's just refactoring.
Any objections?
- Heikki
Attachment | Content-Type | Size |
---|---|---|
rm_desc-signature-change-1.patch | text/x-diff | 23.3 KB |
From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
Subject: | Re: rm_desc signature |
Date: | 2014-06-13 11:39:58 |
Message-ID: | 20140613113958.GB30721@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 2014-06-13 14:37:33 +0300, Heikki Linnakangas wrote:
> As part of the WAL-format changing patch I've been working on, I changed the
> signature of the rm_desc function from:
>
> void (*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
> void (*rm_desc) (StringInfo buf, XLogRecord *record);
>
> The WAL-format patch needed that because it added more functions/macros for
> working with XLogRecords, also used by rm_desc routines, but it seems like a
> sensible change anyway. IMHO it was always a bit strange that rm_desc was
> passed the info field and record payload separately.
+1. I've found this annoying in the past.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)2ndquadrant(dot)com> |
Cc: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: rm_desc signature |
Date: | 2014-06-13 12:30:32 |
Message-ID: | CAHGQGwFSMEZW2cX1GBxxzm7EP-kOgH07H_+e=M7EOCWVg5g3-Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Fri, Jun 13, 2014 at 8:39 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-06-13 14:37:33 +0300, Heikki Linnakangas wrote:
>> As part of the WAL-format changing patch I've been working on, I changed the
>> signature of the rm_desc function from:
>>
>> void (*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
>> void (*rm_desc) (StringInfo buf, XLogRecord *record);
>>
>> The WAL-format patch needed that because it added more functions/macros for
>> working with XLogRecords, also used by rm_desc routines, but it seems like a
>> sensible change anyway. IMHO it was always a bit strange that rm_desc was
>> passed the info field and record payload separately.
+1, too.
-/* #define WAL_DEBUG */
+#define WAL_DEBUG
ISTM you just forgot to exclude this change from the patch.
Regards,
--
Fujii Masao
From: | Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> |
---|---|
To: | Andres Freund <andres(at)2ndquadrant(dot)com> |
Cc: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, pgsql-hackers(at)postgreSQL(dot)org |
Subject: | Re: rm_desc signature |
Date: | 2014-06-13 12:33:27 |
Message-ID: | 20140613123327.GR5162@toroid.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
At 2014-06-13 13:39:58 +0200, andres(at)2ndquadrant(dot)com wrote:
>
> > void (*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
> > void (*rm_desc) (StringInfo buf, XLogRecord *record);
> >
> > […]
>
> +1. I've found this annoying in the past.
I like it too. I was just moving some code from pg_xlogdump into another
(new) rm_desc-like callback, and passing in the XLogRecord makes much
more sense to me.
-- Abhijit
From: | Jeff Janes <jeff(dot)janes(at)gmail(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: rm_desc signature |
Date: | 2014-06-17 01:19:31 |
Message-ID: | CAMkU=1x1b3Gd-r1TYT0U6rLDHFxy_+VhJ1W8va05uCiHXfG+XA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Friday, June 13, 2014, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
wrote:
> As part of the WAL-format changing patch I've been working on, I changed
> the signature of the rm_desc function from:
>
> void (*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
> void (*rm_desc) (StringInfo buf, XLogRecord *record);
>
> The WAL-format patch needed that because it added more functions/macros
> for working with XLogRecords, also used by rm_desc routines, but it seems
> like a sensible change anyway. IMHO it was always a bit strange that
> rm_desc was passed the info field and record payload separately.
>
> So I propose to do that change as a separate commit. Per attached. This
> has no functional changes, it's just refactoring.
>
> Any objections?
>
This commit, or a related one, changed the default (i.e. commented out)
nature of:
#define WAL_DEBUG
Cheers,
Jeff
From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
---|---|
To: | Jeff Janes <jeff(dot)janes(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: rm_desc signature |
Date: | 2014-06-17 05:53:45 |
Message-ID: | 539FD7E9.2010800@vmware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 06/17/2014 04:19 AM, Jeff Janes wrote:
> This commit, or a related one, changed the default (i.e. commented out)
> nature of:
>
> #define WAL_DEBUG
Oops. Fixed, thanks!
- Heikki