Thread: Determining client_encoding from client locale

We currently require that you set client_encoding correctly, or you get 
garbage in psql and any other tool using libpq. How about setting 
client_encoding automatically to match the client's locale? We have 
pg_get_encoding_from_locale() function that we can use to extract the 
encoding from LC_CTYPE. We could call that in libpq.

client_encoding defaults to server_encoding, which is correct in the 
typical environment where the client and the server have identical 
locale settings, which I believe is why we don't see more confused users 
on mailing lists. However, a partner of ours was recently bitten by 
this. That was on Windows; I'm not 100% sure if LC_CTYPE is set 
correctly there by default, but this seems like a good idea nevertheless.

We could expand that to datestyle and the user-settable lc_* settings, 
but I don't want to go that far. In case the server lc_ctype/collate 
settings don't match the client's locale, you would end up with mixed 
lc_* settings which might be more confusing than helpful.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com

Heikki Linnakangas wrote:
>
> client_encoding defaults to server_encoding, which is correct in the 
> typical environment where the client and the server have identical 
> locale settings, which I believe is why we don't see more confused 
> users on mailing lists. However, a partner of ours was recently bitten 
> by this. That was on Windows; I'm not 100% sure if LC_CTYPE is set 
> correctly there by default, but this seems like a good idea nevertheless.
>

IIRC Windows locales are not set via the environment. We've had to do 
some special hackery in a few placed to deal with that.

cheers

andrew

On Wednesday 17 June 2009 14:29:26 Heikki Linnakangas wrote:
> We currently require that you set client_encoding correctly, or you get
> garbage in psql and any other tool using libpq. How about setting
> client_encoding automatically to match the client's locale? We have
> pg_get_encoding_from_locale() function that we can use to extract the
> encoding from LC_CTYPE. We could call that in libpq.

I have been requesting that for years, but the Japanese users/developers 
typically objected to that.  I think it's time to relaunch the campain, 
though.

Peter Eisentraut <peter_e@gmx.net> writes:
> On Wednesday 17 June 2009 14:29:26 Heikki Linnakangas wrote:
>> We currently require that you set client_encoding correctly, or you get
>> garbage in psql and any other tool using libpq. How about setting
>> client_encoding automatically to match the client's locale? We have
>> pg_get_encoding_from_locale() function that we can use to extract the
>> encoding from LC_CTYPE. We could call that in libpq.

> I have been requesting that for years, but the Japanese users/developers 
> typically objected to that.  I think it's time to relaunch the campain, 
> though.

I think at least part of the issue is lack of confidence in our code for
extracting an encoding setting from the locale environment.  Do we
really think it's solid now, on all platforms?  The current uses of
pg_get_encoding_from_locale are all designed to put little faith in it,
and what's more it's had exactly zero non-beta field experience.
        regards, tom lane

Heikki Linnakangas wrote:
> We currently require that you set client_encoding correctly, or you get  
> garbage in psql and any other tool using libpq. How about setting  
> client_encoding automatically to match the client's locale? We have  
> pg_get_encoding_from_locale() function that we can use to extract the  
> encoding from LC_CTYPE. We could call that in libpq.

+1

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Wed, Jun 17, 2009 at 4:54 PM, Alvaro
Herrera<alvherre@commandprompt.com> wrote:
> Heikki Linnakangas wrote:
>> We currently require that you set client_encoding correctly, or you get
>> garbage in psql and any other tool using libpq. How about setting
>> client_encoding automatically to match the client's locale? We have
>> pg_get_encoding_from_locale() function that we can use to extract the
>> encoding from LC_CTYPE. We could call that in libpq.
>
> +1

I wonder if isatty() is true and we have terminfo information if
there's a terminfo capability to query the terminal for the correct
encoding.
But yeah, +1 to automatically using the user's current encoding from LC_CTYPE.


-- 
Gregory Stark

Peter Eisentraut <peter_e@gmx.net> wrote:

> On Wednesday 17 June 2009 14:29:26 Heikki Linnakangas wrote:
> > We currently require that you set client_encoding correctly, or you get
> > garbage in psql and any other tool using libpq. How about setting
> > client_encoding automatically to match the client's locale? We have
> > pg_get_encoding_from_locale() function that we can use to extract the
> > encoding from LC_CTYPE. We could call that in libpq.

+1 for psql, but -1 for libpq.

I think automatic determination is good for psql because it is
an end-user application, but is not always acceptable for middlewares.

Please imagine:
   Web Server <- Application Server <- Database Server   ----------    ------------------    ---------------     UTF-8
      Non-UTF8 env.            UTF-8
 

The Application Server might run on non-UTF8 environment
but it should send outputs in UTF8 encoding. Automatic
encoding determination might break existing services.

> I have been requesting that for years, but the Japanese users/developers 
> typically objected to that.  I think it's time to relaunch the campain, 
> though.

I assume that it is not a Japanese-specific problem and just because
they use multiple encodings. Encodings of OSes in Japan are often SJIS
or EUC_JP, but UTF8 is well-used in web-services and databases.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Itagaki Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
> Peter Eisentraut <peter_e@gmx.net> wrote:
>> On Wednesday 17 June 2009 14:29:26 Heikki Linnakangas wrote:
>>> We currently require that you set client_encoding correctly, or you get
>>> garbage in psql and any other tool using libpq. How about setting
>>> client_encoding automatically to match the client's locale? We have
>>> pg_get_encoding_from_locale() function that we can use to extract the
>>> encoding from LC_CTYPE. We could call that in libpq.

> +1 for psql, but -1 for libpq.

What would make sense to me is for libpq to provide the *code* for this,
but then leave it up to the client application whether to actually call
it; if not the behavior stays the same as before.  Aside from
Itagaki-san's objections, that eliminates backwards-compatibility issues
for other applications.
        regards, tom lane

Itagaki Takahiro wrote:
> Peter Eisentraut <peter_e@gmx.net> wrote:
> 
>> On Wednesday 17 June 2009 14:29:26 Heikki Linnakangas wrote:
>>> We currently require that you set client_encoding correctly, or you get
>>> garbage in psql and any other tool using libpq. How about setting
>>> client_encoding automatically to match the client's locale? We have
>>> pg_get_encoding_from_locale() function that we can use to extract the
>>> encoding from LC_CTYPE. We could call that in libpq.
> 
> +1 for psql, but -1 for libpq.
> 
> I think automatic determination is good for psql because it is
> an end-user application, but is not always acceptable for middlewares.
> 
> Please imagine:
> 
>     Web Server <- Application Server <- Database Server
>     ----------    ------------------    ---------------
>       UTF-8         Non-UTF8 env.            UTF-8
> 
> The Application Server might run on non-UTF8 environment
> but it should send outputs in UTF8 encoding. Automatic
> encoding determination might break existing services.

As soon as someone creates a database in non-UTF-8 encoding in the 
cluster, it would stop working anyway. Setting client_encoding=utf8 
manually would be a lot safer in a situation like that.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com

Tom Lane wrote:
> Itagaki Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
> > Peter Eisentraut <peter_e@gmx.net> wrote:
> >> On Wednesday 17 June 2009 14:29:26 Heikki Linnakangas wrote:
> >>> We currently require that you set client_encoding correctly, or you get
> >>> garbage in psql and any other tool using libpq. How about setting
> >>> client_encoding automatically to match the client's locale? We have
> >>> pg_get_encoding_from_locale() function that we can use to extract the
> >>> encoding from LC_CTYPE. We could call that in libpq.
> 
> > +1 for psql, but -1 for libpq.
> 
> What would make sense to me is for libpq to provide the *code* for this,
> but then leave it up to the client application whether to actually call
> it; if not the behavior stays the same as before.  Aside from
> Itagaki-san's objections, that eliminates backwards-compatibility issues
> for other applications.

Added to TODO:
Add code to detect client encoding and locale from the operating systemenvironment    *
http://archives.postgresql.org/pgsql-hackers/2009-06/msg01040.php
 


--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> What would make sense to me is for libpq to provide the *code* for this,
>> but then leave it up to the client application whether to actually call
>> it; if not the behavior stays the same as before.  Aside from
>> Itagaki-san's objections, that eliminates backwards-compatibility issues
>> for other applications.

> Added to TODO:

BTW, something that occurred to me later is that the details of this
could easily be got wrong.  If libpq is indeed told to get
client_encoding from the client environment, it should arrange to do so
*before* opening the connection, and send the encoding request as part
of the startup packet.  The alternative of providing a function to
adjust the encoding for an already-opened connection is inferior for
a couple of reasons:

* extra network round trip required

* we lose any chance at ensuring that connection failure messages come
back in the client's desired encoding.

(The latter business was already discussed a bit IIRC, but I'm too lazy
to check the archives right now.)

So that means that the API for this should probably involve some
addition to the PQconnectdb parameter string, not a separate function.
        regards, tom lane

Here's my first attempt at setting client_encoding automatically from
locale.

It adds a new conninfo parameter to libpq, "client_encoding". If set to
"auto", libpq uses the encoding as returned by
pg_get_encoding_from_locale(). Any other value is passed through to the
server as is.

psql is modified to set "client_encoding=auto", unless overridden by
PGCLIENTENCODING.


BTW, I had to modify psql to use PQconnectdb() instead of
PQsetdblogin(), so that it can pass the extra parameter. I found it a
bit laboursome to construct the conninfo string with proper escaping,
just to have libpq parse and split it into components again. Could we
have a version of PQconnectdb() with an API more suited for setting the
params programmatically? The PQsetdbLogin() approach doesn't scale as
parameters are added/removed in future versions, but we could have
something like this:

PGconn *PQconnectParams(const char **params)

Where "params" is an array with an even number of parameters, forming
key/value pairs. Usage example:

char *connparams[] = {
    "dbname", "mydb",
    "user", username,
    NULL /* terminate with NULL */
};
conn = PQconnectParams(connparams);

This is similar to what I did internally in psql in the attached patch.

Another idea is to use an array of PQconninfoOption structs:

PQconn *PQconnectParams(PQconninfoOption *params);

This would be quite natural since that's the format returned by
PQconnDefaults() and PQconninfoParse(), but a bit more cumbersome to use
in applications that don't use those functions, as in the previous example.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
commit 24f6d68ddd3725c1f9a98c47f7535b2973ffc492
Author: Heikki Linnakangas <heikki@enterprisedb.com>
Date:   Mon Jul 6 16:54:00 2009 +0300

    Add client_encoding conninfo parameter. By specifying special value
    'auto', libpq will determine the encoding from the current locale.

    Modify psql to use the 'auto' mode if PGCLIENTENCODING if not set.

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 86affb0..a5d45b2 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -236,6 +236,19 @@
          </listitem>
         </varlistentry>

+        <varlistentry id="libpq-connect-client-encoding" xreflabel="client_encoding">
+         <term><literal>client_encoding</literal></term>
+         <listitem>
+         <para>
+          Character encoding to use. This sets the <varname>client_encoding</varname>
+          configuration option for this connection. In addition to the values
+          accepted by the corresponding server option, you can use 'auto' to
+          determine the right encoding from the current locale in the client
+          (LC_CTYPE environment variable on Unix systems).
+         </para>
+         </listitem>
+        </varlistentry>
+
         <varlistentry id="libpq-connect-options" xreflabel="options">
          <term><literal>options</literal></term>
          <listitem>
@@ -5871,6 +5884,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
       linkend="libpq-connect-connect-timeout"> connection parameter.
      </para>
     </listitem>
+
+    <listitem>
+     <para>
+      <indexterm>
+       <primary><envar>PGCLIENTENCODING</envar></primary>
+      </indexterm>
+      <envar>PGCLIENTENCODING</envar> behaves the same as <xref
+      linkend="libpq-connect-client-encoding"> connection parameter.
+     </para>
+    </listitem>
    </itemizedlist>
   </para>

@@ -5907,17 +5930,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
     <listitem>
      <para>
       <indexterm>
-       <primary><envar>PGCLIENTENCODING</envar></primary>
-      </indexterm>
-      <envar>PGCLIENTENCODING</envar> sets the default client character
-      set encoding.  (Equivalent to <literal>SET client_encoding TO
-      ...</literal>.)
-     </para>
-    </listitem>
-
-    <listitem>
-     <para>
-      <indexterm>
        <primary><envar>PGGEQO</envar></primary>
       </indexterm>
       <envar>PGGEQO</envar> sets the default mode for the genetic query
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 0955e13..6991e7a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1239,8 +1239,7 @@ do_connect(char *dbname, char *user, char *host, char *port)

     while (true)
     {
-        n_conn = PQsetdbLogin(host, port, NULL, NULL,
-                              dbname, user, password);
+        n_conn = PSQLconnect(host, port, dbname, user, password);

         /* We can immediately discard the password -- no longer needed */
         if (password)
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 6b2de37..a5a0b0a 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -32,6 +32,8 @@ static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
 static bool command_no_begin(const char *query);
 static bool is_select_command(const char *query);

+static char *construct_conninfo(const char * const *optarray);
+
 /*
  * "Safe" wrapper around strdup()
  */
@@ -1538,3 +1540,75 @@ expand_tilde(char **filename)

     return *filename;
 }
+
+/*
+ * Establish a connection. This has an API similar to libpq's PQsetdblogin(),
+ * but we set "client_encoding=auto" if PGCLIENTENCODING environment variable
+ * is not set.
+ */
+PGconn *
+PSQLconnect(const char *host,
+            const char *port,
+            const char *dbname,
+            const char *user,
+            const char *password)
+{
+    char *conninfo;
+    PGconn *ret;
+
+    const char *opts[] = {
+        "host", host,
+        "port", port,
+        "dbname", dbname,
+        "user", user,
+        "password", password,
+        "client_encoding", getenv("PGCLIENTENCODING") ? NULL : "auto",
+        NULL, NULL
+    };
+
+    conninfo = construct_conninfo(opts);
+    ret = PQconnectdb(conninfo);
+    free(conninfo);
+
+    return ret;
+}
+
+
+/*
+ * Given an array of connection option name/value pairs, construct a
+ * conninfo string suitable for PQconnectdb(). The array must be terminated
+ * by a NULL pointer.
+ */
+static char *
+construct_conninfo(const char * const *optarray)
+{
+    PQExpBufferData buf;
+
+    initPQExpBuffer(&buf);
+
+    while(*optarray)
+    {
+        const char *option = optarray[0];
+        const char *value = optarray[1];
+
+        if (value != NULL)
+        {
+            /* write option name */
+            appendPQExpBuffer(&buf, "%s = '", option);
+
+            /* write value, escaping single quotes and backslashes */
+            while(*value)
+            {
+                if (*value == '\'' || *value == '\\')
+                    appendPQExpBufferChar(&buf, '\\');
+                appendPQExpBufferChar(&buf, *(value++));
+            }
+
+            appendPQExpBufferStr(&buf, "' ");
+        }
+
+        optarray+=2;
+    }
+
+    return buf.data;
+}
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index edcb2e5..e962164 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -57,6 +57,12 @@ extern PGresult *PSQLexec(const char *query, bool start_xact);

 extern bool SendQuery(const char *query);

+extern PGconn *PSQLconnect(const char *host,
+                           const char *port,
+                           const char *dbname,
+                           const char *user,
+                           const char *password);
+
 extern bool is_superuser(void);
 extern bool standard_strings(void);
 extern const char *session_username(void);
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 429e5d9..cf65a76 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -172,7 +172,7 @@ main(int argc, char *argv[])
     do
     {
         new_pass = false;
-        pset.db = PQsetdbLogin(options.host, options.port, NULL, NULL,
+        pset.db = PSQLconnect(options.host, options.port,
                     options.action == ACT_LIST_DB && options.dbname == NULL ?
                                "postgres" : options.dbname,
                                options.username, password);
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 70dfd90..c92fbe6 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -154,6 +154,9 @@ static const PQconninfoOption PQconninfoOptions[] = {
     {"port", "PGPORT", DEF_PGPORT_STR, NULL,
     "Database-Port", "", 6},

+    {"client_encoding", "PGCLIENTENCODING", NULL, NULL,
+    "Client-Encoding", "", 10},
+
     /*
      * "tty" is no longer used either, but keep it present for backwards
      * compatibility.
@@ -225,9 +228,6 @@ static const PQEnvironmentOption EnvironmentOptions[] =
     {
         "PGTZ", "timezone"
     },
-    {
-        "PGCLIENTENCODING", "client_encoding"
-    },
     /* internal performance-related settings */
     {
         "PGGEQO", "geqo"
@@ -424,6 +424,8 @@ connectOptions1(PGconn *conn, const char *conninfo)
     conn->pgpass = tmp ? strdup(tmp) : NULL;
     tmp = conninfo_getval(connOptions, "connect_timeout");
     conn->connect_timeout = tmp ? strdup(tmp) : NULL;
+    tmp = conninfo_getval(connOptions, "client_encoding");
+    conn->client_encoding_initial = tmp ? strdup(tmp) : NULL;
     tmp = conninfo_getval(connOptions, "sslmode");
     conn->sslmode = tmp ? strdup(tmp) : NULL;
     tmp = conninfo_getval(connOptions, "sslkey");
@@ -552,6 +554,17 @@ connectOptions2(PGconn *conn)
         conn->sslmode = strdup(DefaultSSLMode);

     /*
+     * Resolve 'auto' client_encoding
+     */
+    if (conn->client_encoding_initial &&
+        strcmp(conn->client_encoding_initial, "auto") == 0)
+    {
+        int encid = pg_get_encoding_from_locale(NULL);
+        free(conn->client_encoding_initial);
+        conn->client_encoding_initial = strdup(pg_encoding_to_char(encid));
+    }
+
+    /*
      * Only if we get this far is it appropriate to try to connect. (We need a
      * state flag, rather than just the boolean result of this function, in
      * case someone tries to PQreset() the PGconn.)
@@ -1843,7 +1856,7 @@ keep_going:                        /* We will come back to here until there is
                 if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
                 {
                     conn->status = CONNECTION_SETENV;
-                    conn->setenv_state = SETENV_STATE_OPTION_SEND;
+                    conn->setenv_state = SETENV_STATE_CLIENT_ENCODING_SEND;
                     conn->next_eo = EnvironmentOptions;
                     return PGRES_POLLING_WRITING;
                 }
@@ -3649,6 +3662,13 @@ PQsetClientEncoding(PGconn *conn, const char *encoding)
     if (!encoding)
         return -1;

+    /* resolve special 'auto' value from the locale */
+    if (strcmp(encoding, "auto") == 0)
+    {
+        int encid = pg_get_encoding_from_locale(NULL);
+        encoding = pg_encoding_to_char(encid);
+    }
+
     /* check query buffer overflow */
     if (sizeof(qbuf) < (sizeof(query) + strlen(encoding)))
         return -1;
diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c
index 87ba65b..8d45706 100644
--- a/src/interfaces/libpq/fe-protocol2.c
+++ b/src/interfaces/libpq/fe-protocol2.c
@@ -58,6 +58,7 @@ pqSetenvPoll(PGconn *conn)
     switch (conn->setenv_state)
     {
             /* These are reading states */
+        case SETENV_STATE_CLIENT_ENCODING_WAIT:
         case SETENV_STATE_OPTION_WAIT:
         case SETENV_STATE_QUERY1_WAIT:
         case SETENV_STATE_QUERY2_WAIT:
@@ -74,6 +75,7 @@ pqSetenvPoll(PGconn *conn)
             }

             /* These are writing states, so we just proceed. */
+        case SETENV_STATE_CLIENT_ENCODING_SEND:
         case SETENV_STATE_OPTION_SEND:
         case SETENV_STATE_QUERY1_SEND:
         case SETENV_STATE_QUERY2_SEND:
@@ -98,6 +100,34 @@ pqSetenvPoll(PGconn *conn)
     {
         switch (conn->setenv_state)
         {
+            case SETENV_STATE_CLIENT_ENCODING_SEND:
+                {
+                    char        setQuery[100];    /* note length limit in
+                                                 * sprintf below */
+                    const char *val = conn->client_encoding_initial;
+
+                    if (val)
+                    {
+                        if (pg_strcasecmp(val, "default") == 0)
+                            sprintf(setQuery, "SET client_encoding = DEFAULT");
+                        else
+                            sprintf(setQuery, "SET client_encoding = '%.60s'",
+                                    val);
+#ifdef CONNECTDEBUG
+                        fprintf(stderr,
+                                "Sending client_encoding with %s\n",
+                                setQuery);
+#endif
+                        if (!PQsendQuery(conn, setQuery))
+                            goto error_return;
+
+                        conn->setenv_state = SETENV_STATE_CLIENT_ENCODING_WAIT;
+                    }
+                    else
+                        conn->setenv_state = SETENV_STATE_OPTION_SEND;
+                    break;
+                }
+
             case SETENV_STATE_OPTION_SEND:
                 {
                     /*
@@ -142,6 +172,31 @@ pqSetenvPoll(PGconn *conn)
                     break;
                 }

+            case SETENV_STATE_CLIENT_ENCODING_WAIT:
+                {
+                    if (PQisBusy(conn))
+                        return PGRES_POLLING_READING;
+
+                    res = PQgetResult(conn);
+
+                    if (res)
+                    {
+                        if (PQresultStatus(res) != PGRES_COMMAND_OK)
+                        {
+                            PQclear(res);
+                            goto error_return;
+                        }
+                        PQclear(res);
+                        /* Keep reading until PQgetResult returns NULL */
+                    }
+                    else
+                    {
+                        /* Query finished, so send the next option */
+                        conn->setenv_state = SETENV_STATE_OPTION_SEND;
+                    }
+                    break;
+                }
+
             case SETENV_STATE_OPTION_WAIT:
                 {
                     if (PQisBusy(conn))
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 9102b61..a01ace7 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1920,6 +1920,15 @@ build_startup_packet(const PGconn *conn, char *packet,
             strcpy(packet + packet_len, conn->pgoptions);
         packet_len += strlen(conn->pgoptions) + 1;
     }
+    if (conn->client_encoding_initial && conn->client_encoding_initial[0])
+    {
+        if (packet)
+            strcpy(packet + packet_len, "client_encoding");
+        packet_len += strlen("client_encoding") + 1;
+        if (packet)
+            strcpy(packet + packet_len, conn->client_encoding_initial);
+        packet_len += strlen(conn->client_encoding_initial) + 1;
+    }

     /* Add any environment-driven GUC settings needed */
     for (next_eo = options; next_eo->envName; next_eo++)
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index ff9e6b8..1d036ba 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -235,6 +235,8 @@ typedef enum
 /* (this is used only for 2.0-protocol connections) */
 typedef enum
 {
+    SETENV_STATE_CLIENT_ENCODING_SEND,    /* About to send an Environment Option */
+    SETENV_STATE_CLIENT_ENCODING_WAIT,    /* Waiting for above send to complete */
     SETENV_STATE_OPTION_SEND,    /* About to send an Environment Option */
     SETENV_STATE_OPTION_WAIT,    /* Waiting for above send to complete */
     SETENV_STATE_QUERY1_SEND,    /* About to send a status query */
@@ -294,6 +296,7 @@ struct pg_conn
     char       *pgtty;            /* tty on which the backend messages is
                                  * displayed (OBSOLETE, NOT USED) */
     char       *connect_timeout;    /* connection timeout (numeric string) */
+    char       *client_encoding_initial; /* encoding to use */
     char       *pgoptions;        /* options to start the backend with */
     char       *dbName;            /* database name */
     char       *pguser;            /* Postgres username and password, if any */

On Mon, Jul 6, 2009 at 10:00 AM, Heikki
Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:
> Here's my first attempt at setting client_encoding automatically from
> locale.
>
> It adds a new conninfo parameter to libpq, "client_encoding". If set to
> "auto", libpq uses the encoding as returned by
> pg_get_encoding_from_locale(). Any other value is passed through to the
> server as is.
>

i was trying to test this and make a simple program based on the first
libpq example that only shows client_encoding

this little test compiles fine until i applied your patch :(

postgres@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
test-libpq.o -L/usr/local/pgsql/head/lib -lpq
/usr/local/pgsql/head/lib/libpq.so: undefined reference to
`pg_get_encoding_from_locale'
collect2: ld returned 1 exit status

just in case i attached the test program.

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

Jaime Casanova wrote:

> this little test compiles fine until i applied your patch :(
> 
> postgres@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
> test-libpq.o -L/usr/local/pgsql/head/lib -lpq
> /usr/local/pgsql/head/lib/libpq.so: undefined reference to
> `pg_get_encoding_from_locale'

Do you have an older version of libpq.so around?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Wed, Jul 22, 2009 at 7:30 PM, Alvaro
Herrera<alvherre@commandprompt.com> wrote:
> Jaime Casanova wrote:
>
>> this little test compiles fine until i applied your patch :(
>>
>> postgres@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
>> test-libpq.o -L/usr/local/pgsql/head/lib -lpq
>> /usr/local/pgsql/head/lib/libpq.so: undefined reference to
>> `pg_get_encoding_from_locale'
>
> Do you have an older version of libpq.so around?
>

the one that installed with 8.4.0 but i thougth that when you specify
-L to gcc you're telling it where to pick libraries from, no?

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Wed, Jul 22, 2009 at 9:58 PM, Jaime
Casanova<jcasanov@systemguards.com.ec> wrote:
> On Wed, Jul 22, 2009 at 7:30 PM, Alvaro
> Herrera<alvherre@commandprompt.com> wrote:
>> Jaime Casanova wrote:
>>
>>> this little test compiles fine until i applied your patch :(
>>>
>>> postgres@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
>>> test-libpq.o -L/usr/local/pgsql/head/lib -lpq
>>> /usr/local/pgsql/head/lib/libpq.so: undefined reference to
>>> `pg_get_encoding_from_locale'
>>
>> Do you have an older version of libpq.so around?
>>
>
> the one that installed with 8.4.0 but i thougth that when you specify
> -L to gcc you're telling it where to pick libraries from, no?
>

more to the point when i used unpatched 8.5 tree it works just fine


--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Thursday 23 July 2009 02:29:23 Jaime Casanova wrote:
> this little test compiles fine until i applied your patch :(
>
> postgres@casanova1:~/pg_releases/pgtests$ gcc -o test-libpq
> test-libpq.o -L/usr/local/pgsql/head/lib -lpq
> /usr/local/pgsql/head/lib/libpq.so: undefined reference to
> `pg_get_encoding_from_locale'
> collect2: ld returned 1 exit status

libpq fails to link in chklocale.c.

Jaime Casanova <jcasanov@systemguards.com.ec> writes:
> On Wed, Jul 22, 2009 at 7:30 PM, Alvaro
> Herrera<alvherre@commandprompt.com> wrote:
>> Do you have an older version of libpq.so around?

> the one that installed with 8.4.0 but i thougth that when you specify
> -L to gcc you're telling it where to pick libraries from, no?

On most Linux systems, -L doesn't have any effect on what happens at
runtime --- the dynamic linker's search path will determine that.
Try "ldd" on the executable to see which shlibs really get picked up.
        regards, tom lane

On Thu, Jul 23, 2009 at 11:02 AM, Tom Lane<tgl@sss.pgh.pa.us> wrote:
>
> On most Linux systems, -L doesn't have any effect on what happens at
> runtime --- the dynamic linker's search path will determine that.
> Try "ldd" on the executable to see which shlibs really get picked up.
>

yeah! it's using the one that ships with 8.4.0

postgres@casanova1:~/pg_releases/pgtests$ ldd test-libpq           [...other no related libraries...]libpq.so.5 =>
/opt/PostgreSQL/8.4/lib/libpq.so.5(0x00007f7ef6db2000) 

The only way i can compile with the patched version of libpq is with this
gcc -o test-libpq test-libpq.o -L../pgsql/src/port -lpgport
-L../pgsql/src/interfaces/libpq -lpq -L../pgsql/src/port
-Wl,--as-needed -Wl,-rpath,'/usr/local/pgsql/head/lib' -lpgport

BTW, i can compile with the unpatched version if i add -lpgport (seems
like this patch is adding a dependency)
gcc -o test-libpq test-libpq.o -L/usr/local/pgsql/head/lib -lpq -lpgport

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Thursday 23 July 2009 20:16:39 Jaime Casanova wrote:
> On Thu, Jul 23, 2009 at 11:02 AM, Tom Lane<tgl@sss.pgh.pa.us> wrote:
> > On most Linux systems, -L doesn't have any effect on what happens at
> > runtime --- the dynamic linker's search path will determine that.
> > Try "ldd" on the executable to see which shlibs really get picked up.
>
> yeah! it's using the one that ships with 8.4.0
>
> postgres@casanova1:~/pg_releases/pgtests$ ldd test-libpq
>         [...other no related libraries...]
>     libpq.so.5 => /opt/PostgreSQL/8.4/lib/libpq.so.5 (0x00007f7ef6db2000)
>
> The only way i can compile with the patched version of libpq is with this
> gcc -o test-libpq test-libpq.o -L../pgsql/src/port -lpgport
> -L../pgsql/src/interfaces/libpq -lpq -L../pgsql/src/port
> -Wl,--as-needed -Wl,-rpath,'/usr/local/pgsql/head/lib' -lpgport
>
> BTW, i can compile with the unpatched version if i add -lpgport (seems
> like this patch is adding a dependency)
> gcc -o test-libpq test-libpq.o -L/usr/local/pgsql/head/lib -lpq -lpgport

Which proves my point, because libpgport includes chkconfig.c.  But this is 
just a workaround.

On Thu, Jul 23, 2009 at 1:24 PM, Peter Eisentraut<peter_e@gmx.net> wrote:
>
> Which proves my point, because libpgport includes chkconfig.c.  But this is
> just a workaround.
>

yeah! actually the problem i had was because we need to add the
-lpgport to use pg_get_encoding_from_locale and that is something that
this patch introduced

the other unrelated problem i had is my little knowledge about the
search path of libraries, the minimun i need to compile the test
program with the correct libpq is this:

gcc -o test-libpq test-libpq.o -Wl,-rpath,'/usr/local/pgsql/head/lib'
-L /usr/local/pgsql/head/lib -lpq -lpgport

so, at least, the second problem is a documentation one,our docs says:
"""When linking the final program, specify the option -lpq so that the
libpq library gets pulled in, as well as the option -Ldirectory to
point the compiler to the directory where the libpq library resides.
(Again, the compiler will search some directories by default.) For
maximum portability, put the -L option before the -lpq option. For
example:

cc -o testprog testprog1.o testprog2.o -L/usr/local/pgsql/lib -lpq
"""

which is clearly not accurate, we also need to add the -Wl,rpath stuff

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Mon, Jul 6, 2009 at 10:00 AM, Heikki
Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:
> Here's my first attempt at setting client_encoding automatically from
> locale.
>

when i apply your patch and try to compile in windows i get this error

dllwrap  -o libpq.dll --dllname libpq.dll  --def ./libpqdll.def
fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o
libpq-events.o md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o
thread.o crypt.o inet_aton.o strlcpy.o getaddrinfo.o open.o
win32error.o snprintf.o win32.o pgsleep.o libpqrc.o pthread-win32.o
-L../../../src/port -lshfolder -lwsock32 -lws2_32 -lsecur32
fe-connect.o: In function
`PQsetClientEncoding':C:/msys/1.0/home/Administrador/pgsql/src/interfaces/libpq/fe-connect.c:3668:
undefined reference to `pg_get_encoding_from_locale'
fe-connect.o: In function
`connectOptions2':C:/msys/1.0/home/Administrador/pgsql/src/interfaces/libpq/fe-connect.c:562:
undefined reference to `pg_get_encoding_from_locale'
collect2: ld returned 1 exit status
c:\mingw\bin\dllwrap.exe: c:\mingw\bin\gcc exited with status 1
make[3]: *** [libpq.dll] Error 1
make[3]: Leaving directory `/home/Administrador/pgsql/src/interfaces/libpq'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/home/Administrador/pgsql/src/interfaces'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/home/Administrador/pgsql/src'
make: *** [all] Error 2


--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Mon, Jul 6, 2009 at 10:00 AM, Heikki
Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:
> Here's my first attempt at setting client_encoding automatically from
> locale.
>

Sorry for the many mails on this issue.. i will do a recolect of my findings:

1) it introduces a dependency for -lpgport when compiling a client
that uses libpq   http://archives.postgresql.org/pgsql-hackers/2009-07/msg01511.php

2) It doesn't compile in windows   http://archives.postgresql.org/pgsql-hackers/2009-07/msg01515.php

3) why do you need to modify psql at all? i think you need to send the
patch with the api change first and the a second patch that changes
client app that can use it

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Fri, Jul 24, 2009 at 04:12, Jaime
Casanova<jcasanov@systemguards.com.ec> wrote:
> On Mon, Jul 6, 2009 at 10:00 AM, Heikki
> Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:
>> Here's my first attempt at setting client_encoding automatically from
>> locale.
>>
>
> Sorry for the many mails on this issue.. i will do a recolect of my findings:
>
> 1) it introduces a dependency for -lpgport when compiling a client
> that uses libpq
>    http://archives.postgresql.org/pgsql-hackers/2009-07/msg01511.php

For other parts of libpgport that are needed, we pull in the
individual source files. We specifically *don't* link libpq with
libpgport, for a reason. There's a comment in the Makefile that
explains why.


-- Magnus HaganderSelf: http://www.hagander.net/Work: http://www.redpill-linpro.com/

On Fri, Jul 24, 2009 at 2:23 AM, Magnus Hagander<magnus@hagander.net> wrote:
>>
>> 1) it introduces a dependency for -lpgport when compiling a client
>> that uses libpq
>>    http://archives.postgresql.org/pgsql-hackers/2009-07/msg01511.php
>
> For other parts of libpgport that are needed, we pull in the
> individual source files. We specifically *don't* link libpq with
> libpgport, for a reason. There's a comment in the Makefile that
> explains why.
>

ok, attached a version that modifies src/interfaces/libpq/Makefile to
push chklocale.o and eliminate the dependency on libpgport, this
change also fixes the compile problem on windows

still, i'm not sure this patch is doing anything useful... i
initialized a cluster with utf8 and my system is using utf8 but when
executing my test script with client_encoding=auto it gets SQL_ASCII

postgres@casanova1:~/pg_releases/pgtests$ locale
LANG=es_EC.UTF-8
LC_CTYPE="es_EC.UTF-8"
LC_NUMERIC="es_EC.UTF-8"
LC_TIME="es_EC.UTF-8"
LC_COLLATE="es_EC.UTF-8"
LC_MONETARY="es_EC.UTF-8"
LC_MESSAGES="es_EC.UTF-8"
LC_PAPER="es_EC.UTF-8"
LC_NAME="es_EC.UTF-8"
LC_ADDRESS="es_EC.UTF-8"
LC_TELEPHONE="es_EC.UTF-8"
LC_MEASUREMENT="es_EC.UTF-8"
LC_IDENTIFICATION="es_EC.UTF-8"
LC_ALL=
postgres@casanova1:~/pg_releases/pgtests$ ./test-libpq
'dbname=postgres port=54329 client_encoding=auto'
client_encoding: SQL_ASCII

and when executing the same script compiled in windows i get an error,
it doesn't recognize the client_encoding option...

$ ./test-libpq.exe "dbname=postgres user=postgres host=192.168.204.101
port=54329 client_encoding=latin1"
Connection to database failed: invalid connection option "client_encoding"


--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

(I finally got a chance to get back to this...)

Jaime Casanova wrote:
> ok, attached a version that modifies src/interfaces/libpq/Makefile to
> push chklocale.o and eliminate the dependency on libpgport, this
> change also fixes the compile problem on windows

Thanks!

> still, i'm not sure this patch is doing anything useful... i
> initialized a cluster with utf8 and my system is using utf8 but when
> executing my test script with client_encoding=auto it gets SQL_ASCII
> 
> postgres@casanova1:~/pg_releases/pgtests$ locale
> LANG=es_EC.UTF-8
> LC_CTYPE="es_EC.UTF-8"
> LC_NUMERIC="es_EC.UTF-8"
> LC_TIME="es_EC.UTF-8"
> LC_COLLATE="es_EC.UTF-8"
> LC_MONETARY="es_EC.UTF-8"
> LC_MESSAGES="es_EC.UTF-8"
> LC_PAPER="es_EC.UTF-8"
> LC_NAME="es_EC.UTF-8"
> LC_ADDRESS="es_EC.UTF-8"
> LC_TELEPHONE="es_EC.UTF-8"
> LC_MEASUREMENT="es_EC.UTF-8"
> LC_IDENTIFICATION="es_EC.UTF-8"
> LC_ALL=
> postgres@casanova1:~/pg_releases/pgtests$ ./test-libpq
> 'dbname=postgres port=54329 client_encoding=auto'
> client_encoding: SQL_ASCII
> 
> and when executing the same script compiled in windows i get an error,
> it doesn't recognize the client_encoding option...
> 
> $ ./test-libpq.exe "dbname=postgres user=postgres host=192.168.204.101
> port=54329 client_encoding=latin1"
> Connection to database failed: invalid connection option "client_encoding"

Hmm, are you sure you the right version of libpq is being loaded at
runtime? What does "ldd ./test-libpq" say?

--  Heikki Linnakangas EnterpriseDB   http://www.enterprisedb.com

On Tue, Aug 18, 2009 at 6:49 AM, Heikki
Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:
>
> Hmm, are you sure you the right version of libpq is being loaded at
> runtime? What does "ldd ./test-libpq" say?
>

i have to rebuild with the patch on linux and windows and i'm not sure
i will have time until friday...

once said that, in linux i'm very sure it was running with the right
version of libpq... i checked with ldd after my original problems
compiling the test program here:
http://archives.postgresql.org/pgsql-hackers/2009-07/msg01496.php
http://archives.postgresql.org/pgsql-hackers/2009-07/msg01501.php

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Wed, Aug 19, 2009 at 11:08 AM, Jaime
Casanova<jcasanov@systemguards.com.ec> wrote:
> On Tue, Aug 18, 2009 at 6:49 AM, Heikki
> Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:
>>
>> Hmm, are you sure you the right version of libpq is being loaded at
>> runtime? What does "ldd ./test-libpq" say?
>>
>

attached the results of ldd and the result of the test script for
"client_encoding=auto" and "client_encoding=latin1", seems like it's
trying to use auto as an encoding and when it fails takes SQL_ASCII

the same results for windows (i used dependency walker to be sure i
was using the right libpq.dll)

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

Did this patch go anywhere?  Is it a TODO?

---------------------------------------------------------------------------

Heikki Linnakangas wrote:
> Here's my first attempt at setting client_encoding automatically from
> locale.
> 
> It adds a new conninfo parameter to libpq, "client_encoding". If set to
> "auto", libpq uses the encoding as returned by
> pg_get_encoding_from_locale(). Any other value is passed through to the
> server as is.
> 
> psql is modified to set "client_encoding=auto", unless overridden by
> PGCLIENTENCODING.
> 
> 
> BTW, I had to modify psql to use PQconnectdb() instead of
> PQsetdblogin(), so that it can pass the extra parameter. I found it a
> bit laboursome to construct the conninfo string with proper escaping,
> just to have libpq parse and split it into components again. Could we
> have a version of PQconnectdb() with an API more suited for setting the
> params programmatically? The PQsetdbLogin() approach doesn't scale as
> parameters are added/removed in future versions, but we could have
> something like this:
> 
> PGconn *PQconnectParams(const char **params)
> 
> Where "params" is an array with an even number of parameters, forming
> key/value pairs. Usage example:
> 
> char *connparams[] = {
>     "dbname", "mydb",
>     "user", username,
>     NULL /* terminate with NULL */
> };
> conn = PQconnectParams(connparams);
> 
> This is similar to what I did internally in psql in the attached patch.
> 
> Another idea is to use an array of PQconninfoOption structs:
> 
> PQconn *PQconnectParams(PQconninfoOption *params);
> 
> This would be quite natural since that's the format returned by
> PQconnDefaults() and PQconninfoParse(), but a bit more cumbersome to use
> in applications that don't use those functions, as in the previous example.
> 
> -- 
>   Heikki Linnakangas
>   EnterpriseDB   http://www.enterprisedb.com


--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.comPG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard
drive,Christ can be your backup. +
 

On Wed, Feb 24, 2010 at 11:07 AM, Bruce Momjian <bruce@momjian.us> wrote:
>
> Did this patch go anywhere?  Is it a TODO?
>

There were problems with that patch, maybe Heikki will review it again
for 9.1 but for now it's already a TODO, it's in the "Multi-Language
Support" section

Set client encoding based on the client operating system encoding
   Currently client_encoding is set in postgresql.conf, which
defaults to the server encoding.
       * Re: [GENERAL] invalid byte sequence ?

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On lör, 2009-07-25 at 01:41 -0500, Jaime Casanova wrote:
> On Fri, Jul 24, 2009 at 2:23 AM, Magnus Hagander<magnus@hagander.net> wrote:
> >>
> >> 1) it introduces a dependency for -lpgport when compiling a client
> >> that uses libpq
> >>    http://archives.postgresql.org/pgsql-hackers/2009-07/msg01511.php
> >
> > For other parts of libpgport that are needed, we pull in the
> > individual source files. We specifically *don't* link libpq with
> > libpgport, for a reason. There's a comment in the Makefile that
> > explains why.
> >
>
> ok, attached a version that modifies src/interfaces/libpq/Makefile to
> push chklocale.o and eliminate the dependency on libpgport, this
> change also fixes the compile problem on windows

I have adjusted your old patch for the current tree, and it seems to
work.  I think it was just forgotten last time because the move to
PQconnectdbParams had to happen first.  But I'll throw it back into the
ring now.

On 14.01.2011 20:12, Peter Eisentraut wrote:
> I have adjusted your old patch for the current tree, and it seems to
> work.  I think it was just forgotten last time because the move to
> PQconnectdbParams had to happen first.  But I'll throw it back into the
> ring now.

Hello,

maybe i missed pre-discussion but ...

I miss considering auto-detect of file encoding.

A simple example:
$ psql -f dump.sql db

What happens when dump.sql is written by using
another encoding?

I just would expect that when client encoding is detected
by automatism that it also will be detected when I use
files.

My own experience from "the others" is that users
more often forget to think about encoding when they
deal with files as when they type in the client.

Anyway, the code itself seems ok.
I just would recommend to document that the client encoding
should be checked from the user anyway. Just to make sure that
it is not our fault, when there is a libc bug.

Just my 2ct,

Susanne

-- 
Susanne Ebrecht - 2ndQuadrant
PostgreSQL Development, 24x7 Support, Training and Services
www.2ndQuadrant.com

On 17 January 2011 12:58, Susanne Ebrecht <susanne@2ndquadrant.com> wrote:
> Hello,
>
> maybe i missed pre-discussion but ...
>
> I miss considering auto-detect of file encoding.
>
> A simple example:
> $ psql -f dump.sql db
>
> What happens when dump.sql is written by using
> another encoding?

That doesn't tend to be much of a problem in practice because pg_dump
will have the dump SET client_encoding as appropriate from the DB
encoding.

-- 
Regards,
Peter Geoghegan

On 17.01.2011 14:26, Peter Geoghegan wrote:
> On 17 January 2011 12:58, Susanne Ebrecht<susanne@2ndquadrant.com>  wrote:
>> Hello,
>>
>> maybe i missed pre-discussion but ...
>>
>> I miss considering auto-detect of file encoding.
>>
>> A simple example:
>> $ psql -f dump.sql db
>>
>> What happens when dump.sql is written by using
>> another encoding?
> That doesn't tend to be much of a problem in practice because pg_dump
> will have the dump SET client_encoding as appropriate from the DB
> encoding.

Ok, naming it dump.sql was confusing. My fault.
I didn't thought about pg_dump dump files here.
I more thought about files that came out of editors using mad encoding
and maybe then also were created on Windows and then copied to
Unix for import.

Written on little endian, copied to big endian and so on.

Susanne

-- 
Susanne Ebrecht - 2ndQuadrant
PostgreSQL Development, 24x7 Support, Training and Services
www.2ndQuadrant.com

On mån, 2011-01-17 at 14:59 +0100, Susanne Ebrecht wrote:
> I didn't thought about pg_dump dump files here.
> I more thought about files that came out of editors using mad encoding
> and maybe then also were created on Windows and then copied to
> Unix for import.
> 
> Written on little endian, copied to big endian and so on.

That may be worth investigating, but I don't think it's related to the
present patch.

On 17.01.2011 20:18, Peter Eisentraut wrote:
> That may be worth investigating, but I don't think it's related to the
> present patch.
>

As I already said - not at all.
The patch was ok for me.

Susanne

-- 
Susanne Ebrecht - 2ndQuadrant
PostgreSQL Development, 24x7 Support, Training and Services
www.2ndQuadrant.com

Greetings,

* Peter Eisentraut (peter_e@gmx.net) wrote:
> I have adjusted your old patch for the current tree, and it seems to
> work.  I think it was just forgotten last time because the move to
> PQconnectdbParams had to happen first.  But I'll throw it back into the
> ring now.

Right off the bat, I don't like that you removed the references to SET
client_encoding from the documentation, that strikes me as a good thing
to keep, though it could go under the client_encoding varname
documentation that you added.

Also, do we really need a new set of states for this..?  I would have
thought, just reading through the patch, that we could use the existing
OPTION_SEND/OPTION_WAIT states..

I'll be going through the patch in more detail, testing, etc, and will
probably go ahead and do the documentation/comment updates myself,
unless someone cares.
Thanks,
    Stephen

On lör, 2011-01-29 at 11:50 -0500, Stephen Frost wrote:
> Greetings,
> 
> * Peter Eisentraut (peter_e@gmx.net) wrote:
> > I have adjusted your old patch for the current tree, and it seems to
> > work.  I think it was just forgotten last time because the move to
> > PQconnectdbParams had to happen first.  But I'll throw it back into the
> > ring now.
> 
> Right off the bat, I don't like that you removed the references to SET
> client_encoding from the documentation, that strikes me as a good thing
> to keep, though it could go under the client_encoding varname
> documentation that you added.

I don't follow completely.  What was changed was that instead of
documenting that PGCLIENTENCODING is equivalent to SET client_encoding,
it was changed to say that PGCLIENTENCODING corresponds to the
client_encoding connection option, and the client_encoding connection
option documents that it is similar to the client_encoding server
parameter.  Maybe some wordsmithing could be applied, but I don't think
any information was actually removed.

> Also, do we really need a new set of states for this..?  I would have
> thought, just reading through the patch, that we could use the existing
> OPTION_SEND/OPTION_WAIT states..

Don't know.  Maybe Heikki could comment; he wrote that initially.

> I'll be going through the patch in more detail, testing, etc, and will
> probably go ahead and do the documentation/comment updates myself,
> unless someone cares.

Sounds good to me.

On 29.01.2011 19:23, Peter Eisentraut wrote:
>> >  Also, do we really need a new set of states for this..?  I would have
>> >  thought, just reading through the patch, that we could use the existing
>> >  OPTION_SEND/OPTION_WAIT states..
> Don't know.  Maybe Heikki could comment; he wrote that initially.

The other options send by the OPTION_SEND/WAIT machinery come from 
environment variables, there's a getenv() call where the 
SETENV_STATE_OPTION_SEND state is handled. It would be a bit awkward to 
shoehorn client_encoding into that.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com

Ibrar,

* Ibrar Ahmed (ibrar.ahmad@gmail.com) wrote:
> I have reviewed/tested this patch.

Great, thanks for that!

> In my point code should be like this
>
>      *if (conn->client_encoding_initial && conn->client_encoding_initial[0])
>        {
>                if (packet)
>                {
>                        strcpy(packet + packet_len, "client_encoding");
>                        packet_len += strlen("client_encoding") + 1;
>                        strcpy(packet + packet_len,
> conn->client_encoding_initial);
>                        packet_len += strlen(conn->client_encoding_initial) +
> 1;
>               }
>         }*

Makes sense to me, just reading through this email.  Have you tested
this change..?  Could you provide it as an additional patch or a new
patch including the change against head, assuming it still works well in
your testing?

> I will test this patch on Windows and will send results.

That would be great, it's not easy for me to test under Windows.
Thanks!
    Stephen

On Tue, Feb 8, 2011 at 5:05 AM, Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
> Stephen Frost!
> I have modified the code to use ADD_STARTUP_OPTION instead of writing code
> again.
> And  tried the patch on Windows  and Linux and it works for me.

Does this need more review, or should it be marked "Ready for Committer"?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Tue, Feb 8, 2011 at 5:05 AM, Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
> > And  tried the patch on Windows  and Linux and it works for me.
>
> Does this need more review, or should it be marked "Ready for Committer"?

I think it can be marked ready for committer.  Heikki addressed my
questions, though I do think we may end up generalizing those states at
some point later, if we ever end up with a similar argument to this one.
Thanks,
    Stephen

On Fri, Feb 11, 2011 at 10:45 AM, Stephen Frost <sfrost@snowman.net> wrote:
> * Robert Haas (robertmhaas@gmail.com) wrote:
>> On Tue, Feb 8, 2011 at 5:05 AM, Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
>> > And  tried the patch on Windows  and Linux and it works for me.
>>
>> Does this need more review, or should it be marked "Ready for Committer"?
>
> I think it can be marked ready for committer.  Heikki addressed my
> questions, though I do think we may end up generalizing those states at
> some point later, if we ever end up with a similar argument to this one.

OK, done.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On tis, 2011-02-08 at 02:05 -0800, Ibrar Ahmed wrote:
> I have modified the code to use ADD_STARTUP_OPTION instead of writing code
> again.

Committed this version.