Grokbase › Groups › PostgreSQL › pgsql-hackers › September 2010
FAQ

Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Greg Sabino Mullane wrote:
Specifically, LANGUAGE changes the headers of pg_controldata
(but not the actual output, LC_ALL does that). Thanks for the
nudge, I'll get to rewriting some code.
pg_upgrade does this in controldata.c for this exact reason:
/*
* Because we test the pg_resetxlog output strings, it has to be in
* English.
*/
if (getenv("LANG"))
lang = pg_strdup(ctx, getenv("LANG"));
#ifndef WIN32
putenv(pg_strdup(ctx, "LANG=C"));
#else
SetEnvironmentVariableA("LANG", "C");
#endif
You do realize that's far from bulletproof? To be sure that that does
anything, you'd need to set (or unset) LC_ALL and LC_MESSAGES as well.
And I thought Windows spelled it LANGUAGE not LANG ...
I have implemented your suggestion of setting LANG, LANGUAGE, and
LC_MESSAGES based on code in pg_regress.c; that is the only place I see
where we force English, and it certainly has received more testing.

Should I set LC_ALL too? The other variables mentioned in pg_regress.c?
Is this something I should patch to 9.0.X?

Patch attached.

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

+ It's impossible for everything to be true. +

Search Discussions

  • Alvaro Herrera at Sep 1, 2010 at 8:44 pm ⇧

    Excerpts from Bruce Momjian's message of mié sep 01 16:35:18 -0400 2010:

    I have implemented your suggestion of setting LANG, LANGUAGE, and
    LC_MESSAGES based on code in pg_regress.c; that is the only place I see
    where we force English, and it certainly has received more testing.
    I think a real long-term solution is to have a machine-readable format
    for pg_controldata, perhaps something very simple like

    $ pg_controldata --machine
    PGCONTROL_VERSION_NUMBER=903
    CATALOG_VERSION_NUMBER=201008051
    DATABASE_SYSIDENTIFIER=5504177303240039672
    etc

    This wouldn't be subject to translation and thus much easier to process.

    --
    Álvaro Herrera <alvherre@commandprompt.com>
    The PostgreSQL Company - Command Prompt, Inc.
    PostgreSQL Replication, Consulting, Custom Development, 24x7 support
  • Tom Lane at Sep 1, 2010 at 8:51 pm ⇧

    Alvaro Herrera writes:
    Excerpts from Bruce Momjian's message of mié sep 01 16:35:18 -0400 2010:
    I have implemented your suggestion of setting LANG, LANGUAGE, and
    LC_MESSAGES based on code in pg_regress.c; that is the only place I see
    where we force English, and it certainly has received more testing.
    I think a real long-term solution is to have a machine-readable format
    for pg_controldata, perhaps something very simple like
    $ pg_controldata --machine
    PGCONTROL_VERSION_NUMBER=903
    CATALOG_VERSION_NUMBER=201008051
    DATABASE_SYSIDENTIFIER=5504177303240039672
    etc
    This wouldn't be subject to translation and thus much easier to process.
    +1. pg_controldata was never written with the idea that its output
    would be read by programs. If we're going to start doing that, we
    should provide an output format that's suitable, not try to work around
    it in the callers.

    However, that's something for 9.1 and beyond. Bruce's immediate problem
    is what to do in pg_upgrade in 9.0, and there I concur that he should
    duplicate what pg_regress is doing.

    regards, tom lane
  • Bruce Momjian at Sep 1, 2010 at 10:39 pm ⇧

    Tom Lane wrote:
    Alvaro Herrera <alvherre@commandprompt.com> writes:
    Excerpts from Bruce Momjian's message of mié sep 01 16:35:18 -0400 2010:
    I have implemented your suggestion of setting LANG, LANGUAGE, and
    LC_MESSAGES based on code in pg_regress.c; that is the only place I see
    where we force English, and it certainly has received more testing.
    I think a real long-term solution is to have a machine-readable format
    for pg_controldata, perhaps something very simple like
    $ pg_controldata --machine
    PGCONTROL_VERSION_NUMBER=903
    CATALOG_VERSION_NUMBER=201008051
    DATABASE_SYSIDENTIFIER=5504177303240039672
    etc
    This wouldn't be subject to translation and thus much easier to process.
    +1. pg_controldata was never written with the idea that its output
    would be read by programs. If we're going to start doing that, we
    should provide an output format that's suitable, not try to work around
    it in the callers.

    However, that's something for 9.1 and beyond. Bruce's immediate problem
    is what to do in pg_upgrade in 9.0, and there I concur that he should
    duplicate what pg_regress is doing.
    OK, here is a patch that sets all the variables that pg_regress.c sets.

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

    + It's impossible for everything to be true. +
  • Tom Lane at Sep 1, 2010 at 11:16 pm ⇧

    Bruce Momjian writes:
    Tom Lane wrote:
    However, that's something for 9.1 and beyond. Bruce's immediate problem
    is what to do in pg_upgrade in 9.0, and there I concur that he should
    duplicate what pg_regress is doing.
    OK, here is a patch that sets all the variables that pg_regress.c sets.
    I certainly hope that pg_regress isn't freeing the strings it passes
    to putenv() ...

    regards, tom lane
  • Bruce Momjian at Sep 1, 2010 at 11:56 pm ⇧

    Tom Lane wrote:
    Bruce Momjian <bruce@momjian.us> writes:
    Tom Lane wrote:
    However, that's something for 9.1 and beyond. Bruce's immediate problem
    is what to do in pg_upgrade in 9.0, and there I concur that he should
    duplicate what pg_regress is doing.
    OK, here is a patch that sets all the variables that pg_regress.c sets.
    I certainly hope that pg_regress isn't freeing the strings it passes
    to putenv() ...
    pg_regress does not restore these settings (it says with C/English) so
    the code is different.

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

    + It's impossible for everything to be true. +
  • Tom Lane at Sep 2, 2010 at 12:08 am ⇧

    Bruce Momjian writes:
    Tom Lane wrote:
    I certainly hope that pg_regress isn't freeing the strings it passes
    to putenv() ...
    pg_regress does not restore these settings (it says with C/English) so
    the code is different.
    That's not what I'm on about. You're trashing strings that are part of
    the live environment. It might accidentally fail to fail for you, if
    your version of free() doesn't immediately clobber the released storage,
    but it's still broken. Read the putenv() man page.

    + #ifndef WIN32
    + char *envstr = (char *) pg_malloc(ctx, strlen(var) +
    + strlen(val) + 1);
    +
    + sprintf(envstr, "%s=%s", var, val);
    + putenv(envstr);
    + pg_free(envstr);
    ^^^^^^^^^^^^^^^^
    + #else
    + SetEnvironmentVariableA(var, val);
    + #endif

    The fact that there is no such free() in pg_regress is not an oversight
    or shortcut.

    regards, tom lane
  • Bruce Momjian at Sep 4, 2010 at 5:50 pm ⇧

    Tom Lane wrote:
    Bruce Momjian <bruce@momjian.us> writes:
    Tom Lane wrote:
    I certainly hope that pg_regress isn't freeing the strings it passes
    to putenv() ...
    pg_regress does not restore these settings (it says with C/English) so
    the code is different.
    That's not what I'm on about. You're trashing strings that are part of
    the live environment. It might accidentally fail to fail for you, if
    your version of free() doesn't immediately clobber the released storage,
    but it's still broken. Read the putenv() man page.

    + #ifndef WIN32
    + char *envstr = (char *) pg_malloc(ctx, strlen(var) +
    + strlen(val) + 1);
    +
    + sprintf(envstr, "%s=%s", var, val);
    + putenv(envstr);
    + pg_free(envstr);
    ^^^^^^^^^^^^^^^^
    + #else
    + SetEnvironmentVariableA(var, val);
    + #endif

    The fact that there is no such free() in pg_regress is not an oversight
    or shortcut.
    Interesting. I did not know this and it was not clear from my manual
    page or FreeBSD's manual page, but Linux clearly does this.

    Updated patch attached.

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

    + It's impossible for everything to be true. +
  • Bruce Momjian at Sep 7, 2010 at 2:11 pm ⇧

    Bruce Momjian wrote:
    Tom Lane wrote:
    Bruce Momjian <bruce@momjian.us> writes:
    Tom Lane wrote:
    I certainly hope that pg_regress isn't freeing the strings it passes
    to putenv() ...
    pg_regress does not restore these settings (it says with C/English) so
    the code is different.
    That's not what I'm on about. You're trashing strings that are part of
    the live environment. It might accidentally fail to fail for you, if
    your version of free() doesn't immediately clobber the released storage,
    but it's still broken. Read the putenv() man page.

    + #ifndef WIN32
    + char *envstr = (char *) pg_malloc(ctx, strlen(var) +
    + strlen(val) + 1);
    +
    + sprintf(envstr, "%s=%s", var, val);
    + putenv(envstr);
    + pg_free(envstr);
    ^^^^^^^^^^^^^^^^
    + #else
    + SetEnvironmentVariableA(var, val);
    + #endif

    The fact that there is no such free() in pg_regress is not an oversight
    or shortcut.
    Interesting. I did not know this and it was not clear from my manual
    page or FreeBSD's manual page, but Linux clearly does this.

    Updated patch attached.
    Applied to HEAD and 9.0.X. Thanks for the ideas/review.

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

    + It's impossible for everything to be true. +

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedSep 1, '10 at 8:35p
activeSep 7, '10 at 2:11p
posts9
users3
websitepostgresql.org...
irc#postgresql

3 users in discussion

Bruce Momjian: 5 posts Tom Lane: 3 posts Alvaro Herrera: 1 post

Content

People

Support

Translate

site design / logo © 2022 Grokbase