Fix SetClientEncoding() to maintain a cache of previously selected encoding
authorTom Lane
Thu, 2 Apr 2009 17:30:53 +0000 (17:30 +0000)
committerTom Lane
Thu, 2 Apr 2009 17:30:53 +0000 (17:30 +0000)
conversion functions.  This allows transaction rollback to revert to a
previous client_encoding setting without doing fresh catalog lookups.
I believe that this explains and fixes the recent report of "failed to commit
client_encoding" failures.

This bug is present in 8.3.x, but it doesn't seem prudent to back-patch
the fix, at least not till it's had some time for field testing in HEAD.

In passing, remove SetDefaultClientEncoding(), which was used nowhere.

src/backend/utils/mb/mbutils.c
src/include/mb/pg_wchar.h

index f54b7486cbfcb021d7bacdf656b9dc8c67be3833..ce554c52c75ae5b45ab6b783547f8f49f4c791e9 100644 (file)
@@ -1,10 +1,10 @@
 /*
  * This file contains public functions for conversion between
- * client encoding and server internal encoding.
- * (currently mule internal code (mic) is used)
+ * client encoding and server (database) encoding.
+ *
  * Tatsuo Ishii
  *
- * $PostgreSQL: pgsql/src/backend/utils/mb/mbutils.c,v 1.82 2009/03/09 00:01:32 alvherre Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/mb/mbutils.c,v 1.83 2009/04/02 17:30:53 tgl Exp $
  */
 #include "postgres.h"
 
 #define MAX_CONVERSION_GROWTH  4
 
 /*
- * We handle for actual FE and BE encoding setting encoding-identificator
- * and encoding-name too. It prevent searching and conversion from encoding
- * to encoding name in getdatabaseencoding() and other routines.
+ * We maintain a simple linked list caching the fmgr lookup info for the
+ * currently selected conversion functions, as well as any that have been
+ * selected previously in the current session.  (We remember previous
+ * settings because we must be able to restore a previous setting during
+ * transaction rollback, without doing any fresh catalog accesses.)
+ *
+ * Since we'll never release this data, we just keep it in TopMemoryContext.
  */
-static pg_enc2name *ClientEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
-static pg_enc2name *DatabaseEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
+typedef struct ConvProcInfo
+{
+   int         s_encoding;     /* server and client encoding IDs */
+   int         c_encoding;
+   FmgrInfo    to_server_info; /* lookup info for conversion procs */
+   FmgrInfo    to_client_info;
+} ConvProcInfo;
+
+static List *ConvProcList = NIL;   /* List of ConvProcInfo */
 
 /*
- * Caches for conversion function info. These values are allocated in
- * MbProcContext. That context is a child of TopMemoryContext,
- * which allows these values to survive across transactions. See
- * SetClientEncoding() for more details.
+ * These variables point to the currently active conversion functions,
+ * or are NULL when no conversion is needed.
  */
-static MemoryContext MbProcContext = NULL;
 static FmgrInfo *ToServerConvProc = NULL;
 static FmgrInfo *ToClientConvProc = NULL;
 
+/*
+ * These variables track the currently selected FE and BE encodings.
+ */
+static pg_enc2name *ClientEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
+static pg_enc2name *DatabaseEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
+
 /*
  * During backend startup we can't set client encoding because we (a)
  * can't look up the conversion functions, and (b) may not know the database
@@ -70,11 +84,7 @@ int
 SetClientEncoding(int encoding, bool doit)
 {
    int         current_server_encoding;
-   Oid         to_server_proc,
-               to_client_proc;
-   FmgrInfo   *to_server;
-   FmgrInfo   *to_client;
-   MemoryContext oldcontext;
+   ListCell   *lc;
 
    if (!PG_VALID_FE_ENCODING(encoding))
        return -1;
@@ -101,79 +111,117 @@ SetClientEncoding(int encoding, bool doit)
            ClientEncoding = &pg_enc2name_tbl[encoding];
            ToServerConvProc = NULL;
            ToClientConvProc = NULL;
-           if (MbProcContext)
-               MemoryContextReset(MbProcContext);
        }
        return 0;
    }
 
-   /*
-    * If we're not inside a transaction then we can't do catalog lookups, so
-    * fail.  After backend startup, this could only happen if we are
-    * re-reading postgresql.conf due to SIGHUP --- so basically this just
-    * constrains the ability to change client_encoding on the fly from
-    * postgresql.conf.  Which would probably be a stupid thing to do anyway.
-    */
-   if (!IsTransactionState())
-       return -1;
+   if (IsTransactionState())
+   {
+       /*
+        * If we're in a live transaction, it's safe to access the catalogs,
+        * so look up the functions.  We repeat the lookup even if the info
+        * is already cached, so that we can react to changes in the contents
+        * of pg_conversion.
+        */
+       Oid         to_server_proc,
+                   to_client_proc;
+       ConvProcInfo *convinfo;
+       MemoryContext oldcontext;
+
+       to_server_proc = FindDefaultConversionProc(encoding,
+                                                  current_server_encoding);
+       if (!OidIsValid(to_server_proc))
+           return -1;
+       to_client_proc = FindDefaultConversionProc(current_server_encoding,
+                                                  encoding);
+       if (!OidIsValid(to_client_proc))
+           return -1;
 
-   /*
-    * Look up the conversion functions.
-    */
-   to_server_proc = FindDefaultConversionProc(encoding,
-                                              current_server_encoding);
-   if (!OidIsValid(to_server_proc))
-       return -1;
-   to_client_proc = FindDefaultConversionProc(current_server_encoding,
-                                              encoding);
-   if (!OidIsValid(to_client_proc))
-       return -1;
+       /*
+        * Done if not wanting to actually apply setting.
+        */
+       if (!doit)
+           return 0;
 
-   /*
-    * Done if not wanting to actually apply setting.
-    */
-   if (!doit)
-       return 0;
+       /*
+        * Load the fmgr info into TopMemoryContext (could still fail here)
+        */
+       convinfo = (ConvProcInfo *) MemoryContextAlloc(TopMemoryContext,
+                                                      sizeof(ConvProcInfo));
+       convinfo->s_encoding = current_server_encoding;
+       convinfo->c_encoding = encoding;
+       fmgr_info_cxt(to_server_proc, &convinfo->to_server_info,
+                     TopMemoryContext);
+       fmgr_info_cxt(to_client_proc, &convinfo->to_client_info,
+                     TopMemoryContext);
+
+       /* Attach new info to head of list */
+       oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+       ConvProcList = lcons(convinfo, ConvProcList);
+       MemoryContextSwitchTo(oldcontext);
 
-   /* Before loading the new fmgr info, remove the old info, if any */
-   ToServerConvProc = NULL;
-   ToClientConvProc = NULL;
-   if (MbProcContext != NULL)
-   {
-       MemoryContextReset(MbProcContext);
+       /*
+        * Everything is okay, so apply the setting.
+        */
+       ClientEncoding = &pg_enc2name_tbl[encoding];
+       ToServerConvProc = &convinfo->to_server_info;
+       ToClientConvProc = &convinfo->to_client_info;
+
+       /*
+        * Remove any older entry for the same encoding pair (this is just
+        * to avoid memory leakage).
+        */
+       foreach(lc, ConvProcList)
+       {
+           ConvProcInfo *oldinfo = (ConvProcInfo *) lfirst(lc);
+
+           if (oldinfo == convinfo)
+               continue;
+           if (oldinfo->s_encoding == convinfo->s_encoding &&
+               oldinfo->c_encoding == convinfo->c_encoding)
+           {
+               ConvProcList = list_delete_ptr(ConvProcList, oldinfo);
+               pfree(oldinfo);
+               break;          /* need not look further */
+           }
+       }
+
+       return 0;               /* success */
    }
    else
    {
        /*
-        * This is the first time through, so create the context. Make it a
-        * child of TopMemoryContext so that these values survive across
-        * transactions.
+        * If we're not in a live transaction, the only thing we can do
+        * is restore a previous setting using the cache.  This covers all
+        * transaction-rollback cases.  The only case it might not work for
+        * is trying to change client_encoding on the fly by editing
+        * postgresql.conf and SIGHUP'ing.  Which would probably be a stupid
+        * thing to do anyway.
         */
-       MbProcContext = AllocSetContextCreate(TopMemoryContext,
-                                             "MbProcContext",
-                                             ALLOCSET_SMALL_MINSIZE,
-                                             ALLOCSET_SMALL_INITSIZE,
-                                             ALLOCSET_SMALL_MAXSIZE);
-   }
-
-   /* Load the fmgr info into MbProcContext */
-   oldcontext = MemoryContextSwitchTo(MbProcContext);
-   to_server = palloc(sizeof(FmgrInfo));
-   to_client = palloc(sizeof(FmgrInfo));
-   fmgr_info(to_server_proc, to_server);
-   fmgr_info(to_client_proc, to_client);
-   MemoryContextSwitchTo(oldcontext);
+       foreach(lc, ConvProcList)
+       {
+           ConvProcInfo *oldinfo = (ConvProcInfo *) lfirst(lc);
 
-   ClientEncoding = &pg_enc2name_tbl[encoding];
-   ToServerConvProc = to_server;
-   ToClientConvProc = to_client;
+           if (oldinfo->s_encoding == current_server_encoding &&
+               oldinfo->c_encoding == encoding)
+           {
+               if (doit)
+               {
+                   ClientEncoding = &pg_enc2name_tbl[encoding];
+                   ToServerConvProc = &oldinfo->to_server_info;
+                   ToClientConvProc = &oldinfo->to_client_info;
+               }
+               return 0;
+           }
+       }
 
-   return 0;
+       return -1;              /* it's not cached, so fail */
+   }
 }
 
 /*
  * Initialize client encoding if necessary.
- *     called from InitPostgres() once during backend starting up.
+ *     called from InitPostgres() once during backend startup.
  */
 void
 InitializeClientEncoding(void)
@@ -196,7 +244,8 @@ InitializeClientEncoding(void)
 }
 
 /*
- * returns the current client encoding */
+ * returns the current client encoding
+ */
 int
 pg_get_client_encoding(void)
 {
@@ -511,10 +560,9 @@ pg_server_to_client(const char *s, int len)
 /*
  * Perform default encoding conversion using cached FmgrInfo. Since
  * this function does not access database at all, it is safe to call
- * outside transactions. Explicit setting client encoding required
- * before calling this function. Otherwise no conversion is
- * performed.
-*/
+ * outside transactions.  If the conversion has not been set up by
+ * SetClientEncoding(), no conversion is performed.
+ */
 static char *
 perform_default_encoding_conversion(const char *src, int len, bool is_client_to_server)
 {
@@ -919,12 +967,6 @@ pg_bind_textdomain_codeset(const char *domainname, int encoding)
 #endif
 }
 
-void
-SetDefaultClientEncoding(void)
-{
-   ClientEncoding = &pg_enc2name_tbl[GetDatabaseEncoding()];
-}
-
 int
 GetDatabaseEncoding(void)
 {
index 3f93d7bc1287dd98d240533379a58de84181fcc1..5b780db2a2deb1f2d42ac5d6db4fcf15daf02f7b 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/mb/pg_wchar.h,v 1.87 2009/03/09 00:01:32 alvherre Exp $
+ * $PostgreSQL: pgsql/src/include/mb/pg_wchar.h,v 1.88 2009/04/02 17:30:53 tgl Exp $
  *
  * NOTES
  *     This is used both by the backend and by libpq, but should not be
@@ -383,7 +383,6 @@ extern size_t wchar2char(char *to, const wchar_t *from, size_t tolen);
 extern size_t char2wchar(wchar_t *to, size_t tolen, const char *from, size_t fromlen);
 #endif
 
-extern void SetDefaultClientEncoding(void);
 extern int SetClientEncoding(int encoding, bool doit);
 extern void InitializeClientEncoding(void);
 extern int pg_get_client_encoding(void);