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

On Fri, Feb 12, 2016 at 9:18 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> 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(at)joeconway(dot)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.

OK, I see now. I did not consider the case where people would like to
get upgrade from a dev version of 9.6 to the latest 9.6 version, just
the upgrade from a previous major version <= 9.5. Thanks for reminding
that pg_upgrade needs to support that.
--
Michael