Re: NextXID format change (was Re: exposing pg_controldata and pg_config as functions)

On Wed, Feb 10, 2016 at 10:23:41AM +0900, Michael Paquier wrote:
> On Wed, Feb 10, 2016 at 9:57 AM, Joe Conway <mail@joeconway.com> wrote:
> > I'll commit the attached tomorrow if there are no other concerns voiced.
> 
> Just a nitpick regarding this block:
> +           if (strchr(p, '/') != NULL)
> +               p = strchr(p, '/');
> +           /* delimiter changed from '/' to ':' in 9.6 */
> +           else if (GET_MAJOR_VERSION(cluster->major_version) >= 906)
> +               p = strchr(p, ':');
> +           else
> +               p = NULL;
> Changing it as follows would save some instructions because there is
> no need to call strchr an extra time:
> if (GET_MAJOR_VERSION(cluster->major_version) >= 906)
>     p = strchr(p, ':');
> else
>     p = strchr(p, '/');

No, that is not an improvement --- see my previous comment:

> We could get more sophisticated by checking the catalog version number
> where the format was changed, but that doesn't seem worth it, and is
> overly complex because we get the catalog version number from
> pg_controldata, so you would be adding a dependency in ordering of the
> pg_controldata entries.

By testing for '906', you prevent users from using pg_upgrade to go from
one catalog version of 9.6 to a later one.  Few people may want to do
it, but it should work.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +