Re: Why do we let CREATE DATABASE reassign encoding? - Mailing list pgsql-hackers

I wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> AFAIR, the only reason that we haven't disallowed this sort of stuff
>> years and years ago is that people use it; the Japanese in particular.
>> I don't see what is different now.

> What's different now is that 8.4 has already established the principle
> that you have to clone template0 if you want to change the locale of a
> database.  I think this is a good time to establish the same principle
> for encodings.  (Or in other words, if we don't fix it now, when will
> be a better time?)

Attached is a proposed patch (without documentation changes as yet)
for this.  Since the code is already enforcing exact locale match when
cloning a non-template0 database, I just made it act the same for
encoding, without any strange exceptions for SQL_ASCII.

I found that mbregress.sh was already broken by the existing
restrictions, if you try to use it in a database whose default
locale isn't C.  The patch adds switches to fix that.

The patch also incidentally fixes a few ereport's that were missing
errcode values.

Last chance for objections ...

            regards, tom lane

Index: src/backend/commands/dbcommands.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/dbcommands.c,v
retrieving revision 1.223
diff -c -r1.223 dbcommands.c
*** src/backend/commands/dbcommands.c    5 May 2009 23:39:55 -0000    1.223
--- src/backend/commands/dbcommands.c    6 May 2009 00:30:59 -0000
***************
*** 361,367 ****
  #endif
            (encoding == PG_SQL_ASCII && superuser())))
          ereport(ERROR,
!                 (errmsg("encoding %s does not match locale %s",
                          pg_encoding_to_char(encoding),
                          dbctype),
               errdetail("The chosen LC_CTYPE setting requires encoding %s.",
--- 361,368 ----
  #endif
            (encoding == PG_SQL_ASCII && superuser())))
          ereport(ERROR,
!                 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
!                  errmsg("encoding %s does not match locale %s",
                          pg_encoding_to_char(encoding),
                          dbctype),
               errdetail("The chosen LC_CTYPE setting requires encoding %s.",
***************
*** 374,402 ****
  #endif
            (encoding == PG_SQL_ASCII && superuser())))
          ereport(ERROR,
!                 (errmsg("encoding %s does not match locale %s",
                          pg_encoding_to_char(encoding),
                          dbcollate),
               errdetail("The chosen LC_COLLATE setting requires encoding %s.",
                         pg_encoding_to_char(collate_encoding))));

      /*
!      * Check that the new locale is compatible with the source database.
       *
!      * We know that template0 doesn't contain any indexes that depend on
!      * collation or ctype, so template0 can be used as template for
!      * any locale.
       */
      if (strcmp(dbtemplate, "template0") != 0)
      {
          if (strcmp(dbcollate, src_collate) != 0)
              ereport(ERROR,
!                     (errmsg("new collation is incompatible with the collation of the template database (%s)",
src_collate),
                       errhint("Use the same collation as in the template database, or use template0 as template.")));

          if (strcmp(dbctype, src_ctype) != 0)
              ereport(ERROR,
!                     (errmsg("new LC_CTYPE is incompatible with LC_CTYPE of the template database (%s)", src_ctype),
                       errhint("Use the same LC_CTYPE as in the template database, or use template0 as template.")));
      }

--- 375,419 ----
  #endif
            (encoding == PG_SQL_ASCII && superuser())))
          ereport(ERROR,
!                 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
!                  errmsg("encoding %s does not match locale %s",
                          pg_encoding_to_char(encoding),
                          dbcollate),
               errdetail("The chosen LC_COLLATE setting requires encoding %s.",
                         pg_encoding_to_char(collate_encoding))));

      /*
!      * Check that the new encoding and locale settings match the source
!      * database.  We insist on this because we simply copy the source data ---
!      * any non-ASCII data would be wrongly encoded, and any indexes sorted
!      * according to the source locale would be wrong.
       *
!      * However, we assume that template0 doesn't contain any non-ASCII data
!      * nor any indexes that depend on collation or ctype, so template0 can be
!      * used as template for creating a database with any encoding or locale.
       */
      if (strcmp(dbtemplate, "template0") != 0)
      {
+         if (encoding != src_encoding)
+             ereport(ERROR,
+                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                      errmsg("new encoding (%s) is incompatible with the encoding of the template database (%s)",
+                             pg_encoding_to_char(encoding),
+                             pg_encoding_to_char(src_encoding)),
+                      errhint("Use the same encoding as in the template database, or use template0 as template.")));
+
          if (strcmp(dbcollate, src_collate) != 0)
              ereport(ERROR,
!                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
!                      errmsg("new collation (%s) is incompatible with the collation of the template database (%s)",
!                             dbcollate, src_collate),
                       errhint("Use the same collation as in the template database, or use template0 as template.")));

          if (strcmp(dbctype, src_ctype) != 0)
              ereport(ERROR,
!                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
!                      errmsg("new LC_CTYPE (%s) is incompatible with the LC_CTYPE of the template database (%s)",
!                             dbctype, src_ctype),
                       errhint("Use the same LC_CTYPE as in the template database, or use template0 as template.")));
      }

***************
*** 1099,1105 ****
                  continue;

              ereport(ERROR,
!                     (errmsg("some relations of database \"%s\" are already in tablespace \"%s\"",
                              dbname, tblspcname),
                       errhint("You must move them back to the database's default tablespace before using this
command.")));
          }
--- 1116,1123 ----
                  continue;

              ereport(ERROR,
!                     (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
!                      errmsg("some relations of database \"%s\" are already in tablespace \"%s\"",
                              dbname, tblspcname),
                       errhint("You must move them back to the database's default tablespace before using this
command.")));
          }
Index: src/test/mb/mbregress.sh
===================================================================
RCS file: /cvsroot/pgsql/src/test/mb/mbregress.sh,v
retrieving revision 1.9
diff -c -r1.9 mbregress.sh
*** src/test/mb/mbregress.sh    24 Jun 2005 15:11:59 -0000    1.9
--- src/test/mb/mbregress.sh    6 May 2009 00:30:59 -0000
***************
*** 15,21 ****
  fi

  dropdb utf8
! createdb -E UTF8 utf8

  PSQL="psql -n -e -q"
  tests="euc_jp sjis euc_kr euc_cn euc_tw big5 utf8 mule_internal"
--- 15,21 ----
  fi

  dropdb utf8
! createdb -T template0 -l C -E UTF8 utf8

  PSQL="psql -n -e -q"
  tests="euc_jp sjis euc_kr euc_cn euc_tw big5 utf8 mule_internal"
***************
*** 36,42 ****
          unset PGCLIENTENCODING
      else
          dropdb $i >/dev/null 2>&1
!         createdb -E `echo $i | tr 'abcdefghijklmnopqrstuvwxyz' 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'` $i >/dev/null
          $PSQL $i < sql/${i}.sql > results/${i}.out 2>&1
      fi

--- 36,42 ----
          unset PGCLIENTENCODING
      else
          dropdb $i >/dev/null 2>&1
!         createdb -T template0 -l C -E `echo $i | tr 'abcdefghijklmnopqrstuvwxyz' 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'` $i
>/dev/null
          $PSQL $i < sql/${i}.sql > results/${i}.out 2>&1
      fi