Re: pg_upgrade: make the locale comparison more tolerating

Hello pg-hackers!

I tried to look at the problem resulting in changes mentioned here:
http://www.postgresql.org/message-id/20121002155857.GE30089@momjian.us

If the system locale is changed e.g. from en_US.utf8 to en_US.utf-8 before
upgrading the data stack for newer server, pg_upgrade fails. It is quite
awkward to ask users to change locale.conf contents just for upgrade
purposes; is there other known workaround?

I attached patch aimed to remove this hurdle. After its application, the
in-place upgrade finished successfully for me under mentioned conditions,
though I am not 100% sure about its correctness.

Thanks, Pavel

I made the patch to be in context format and I'll add this issue to
CommitFest. Also, some code comments and style was changed to meet better
readability.

Hello,

I started reviewing the patch. Go through the original mail thread to
understand
the need of fix and the actual problem.

http://www.postgresql.org/message-id/20121002155857.GE30089@momjian.us

Patch is using pg_valid_server_encoding() to compare the encoding which
looks
more correct. Did code walk through and it looks good to me. I tried to test
the patch on CentOS and its working fine. I am not quite knowledgeable
about other OS so on that perspective would good to have others view.

Here are the comment about the patch:

.) Patch gets cleanly apply on master ( using patch -p1 )
.) compilation done successful
.) Code walk through and logic looks good
.) Manual testing worked good for me

To test the issue I set the old database locale to en_US.utf8 and for new
database
locale to en_US.utf-8. WIth this when I tried pg_upgrade it failed with
"lc_collate cluster
values do not match: old "en_US.utf8", new "en_US.UTF-8". With the patch
pg_upgrade
running fine.

Regards,

Rushabh, really sorry I have to re-create the patch and thanks a
lot for looking at it!

Looking at the patch once again, I see that there were at least two
problems. Firstly, I used the equivalent_locale function also on the
encoding values. Even if that should not cause bugs (as it should result
in strncasecmp anyway), it was not pretty..

The second problem was assuming that the locale specifier "A" is not
longer then locale specifier B. Comparisons like 'en_US.utf8' with
'en_US_.utf8' would result in success. Bug resulting from this mistake is
not real probably but it is not nice anyway..

Rather cleaning the patch once more, attached,
Pavel

Hi Pavel,

I looked at your latest cleanup patch. Yes its looks more cleaner in term
equivalent_locale & equivalent_encoding separate functions.

I did run the test again on latest patch and all looks good.

Marking it as ready for committer.

Regards,

On Fri, Jan 24, 2014 at 9:49 PM, Pavel Raiskup <praiskup(at)redhat(dot)com> wrote:

> Rushabh, really sorry I have to re-create the patch and thanks a
> lot for looking at it!
>
> Looking at the patch once again, I see that there were at least two
> problems. Firstly, I used the equivalent_locale function also on the
> encoding values. Even if that should not cause bugs (as it should result
> in strncasecmp anyway), it was not pretty..
>
> The second problem was assuming that the locale specifier "A" is not
> longer then locale specifier B. Comparisons like 'en_US.utf8' with
> 'en_US_.utf8' would result in success. Bug resulting from this mistake is
> not real probably but it is not nice anyway..
>
> Rather cleaning the patch once more, attached,
> Pavel
>

--
Rushabh Lathia

Pavel Raiskup <praiskup(at)redhat(dot)com> writes:
> [ 0001-pg_upgrade-make-the-locale-comparison-more-toleratin.patch ]

Committed with minor adjustments (mostly, wordsmithing the comments).

Although this was categorized as a bug fix, I'm not sure it's worth
taking the risk of back-patching. We've not seen very many reports
of problems of this type. Of course, you're free to carry it as a
patch in Red Hat's packages if you differ ...

regards, tom lane