Re: Dump pageinspect to 1.2: page_header with pg_lsn datatype

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