On Wed, Feb 26, 2014 at 5:45 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Robert Haas escribió:
>> On Mon, Feb 24, 2014 at 9:34 AM, Alvaro Herrera
>> <alvherre@2ndquadrant.com> wrote:
>> > Yeah, erroring out seems good enough. Particularly if you add a hint
>> > saying "please upgrade the extension".
>>
>> In past instances where this has come up, we have actually made the
>> .so backward-compatible. See pg_stat_statements in particular. I'd
>> prefer to keep to that precedent here.
>
> But pg_stat_statement is a user tool which is expected to have lots of
> use, and backwards compatibility concerns because of people who might
> have written tools on top of it. Not so with pageinspect. I don't
> think we need to put in the same amount of effort. (Even though,
> really, it's probably not all that difficult to support both cases. I
> just don't see the point.)
Actually a little bit of hacking I noticed that supporting both is as
complicated as supporting only pg_lsn... Here is for example how I can
get page_header to work across versions:
- snprintf(lsnchar, sizeof(lsnchar), "%X/%X",
- (uint32) (lsn >> 32), (uint32) lsn);
- values[0] = CStringGetTextDatum(lsnchar);
+ /*
+ * Do some version-related checks. pageinspect >= 1.2 uses pg_lsn
+ * instead of text when using this function for the LSN field.
+ */
+ if (tupdesc->attrs[0]->atttypid == TEXTOID)
+ {
+ char lsnchar[64];
+ snprintf(lsnchar, sizeof(lsnchar), "%X/%X",
+ (uint32) (lsn >> 32), (uint32) lsn);
+ values[0] = CStringGetTextDatum(lsnchar);
+ }
+ else
+ values[0] = LSNGetDatum(lsn);
In this case an upgraded 9.4 server using pageinspect 1.1 or older
simply goes through the text datatype path... You can find that in the
patch attached.
Regards,
--
Michael
