On 2019-07-13 19:20, Fabien COELHO wrote:
> The second error message about conflicting option could more explicit than 
> a terse "conflicting or redundant options"? The user may expect later 
> options to superseedes earlier options, for instance.

done

> About the pg_dump code, I'm wondering whether it is worth generating 
> LOCALE as it breaks backward compatibility (eg dumping a new db to load it 
> into a older version).

We don't really care about backward compatibility here.  Moving forward,
with ICU and such, we don't want to have to drag around old syntax forever.

> If it is to be generated, I'd do merge the two conditions instead of 
> nesting.
> 
>    if (strlen(collate) > 0 && strcmp(collate, ctype) == 0)
>      // generate LOCALE

done

How about this patch?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From df26107a550b9e1d0933a04dc31f43c43a17b0f7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 22 Jul 2019 20:35:43 +0200
Subject: [PATCH v3] Add CREATE DATABASE LOCALE option

This sets both LC_COLLATE and LC_CTYPE with one option.  Similar
behavior is already supported in initdb, CREATE COLLATION, and
createdb.

Discussion: 
https://www.postgresql.org/message-id/flat/d9d5043a-dc70-da8a-0166-1e218e6e34d4%402ndquadrant.com
---
 doc/src/sgml/ref/create_database.sgml | 25 +++++++++++++++++++++++--
 src/backend/commands/dbcommands.c     | 21 +++++++++++++++++++++
 src/bin/pg_dump/pg_dump.c             | 18 +++++++++++++-----
 src/bin/pg_dump/t/002_pg_dump.pl      |  9 +++++++++
 4 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/ref/create_database.sgml 
b/doc/src/sgml/ref/create_database.sgml
index b2c9e241c2..4014f6703b 100644
--- a/doc/src/sgml/ref/create_database.sgml
+++ b/doc/src/sgml/ref/create_database.sgml
@@ -25,6 +25,7 @@
     [ [ WITH ] [ OWNER [=] <replaceable 
class="parameter">user_name</replaceable> ]
            [ TEMPLATE [=] <replaceable 
class="parameter">template</replaceable> ]
            [ ENCODING [=] <replaceable 
class="parameter">encoding</replaceable> ]
+           [ LOCALE [=] <replaceable class="parameter">locale</replaceable> ]
            [ LC_COLLATE [=] <replaceable 
class="parameter">lc_collate</replaceable> ]
            [ LC_CTYPE [=] <replaceable 
class="parameter">lc_ctype</replaceable> ]
            [ TABLESPACE [=] <replaceable 
class="parameter">tablespace_name</replaceable> ]
@@ -111,6 +112,26 @@ <title>Parameters</title>
        </para>
       </listitem>
      </varlistentry>
+     <varlistentry>
+      <term><replaceable class="parameter">locale</replaceable></term>
+      <listitem>
+       <para>
+        This is a shortcut for setting <symbol>LC_COLLATE</symbol>
+        and <symbol>LC_CTYPE</symbol> at once.  If you specify this,
+        you cannot specify either of those parameters.
+       </para>
+       <tip>
+        <para>
+         The other locale settings <xref linkend="guc-lc-messages"/>, <xref
+         linkend="guc-lc-monetary"/>, <xref linkend="guc-lc-numeric"/>, and
+         <xref linkend="guc-lc-time"/> are not fixed per database and are not
+         set by this command.  If you want to make them the default for a
+         specific database, you can use <literal>ALTER DATABASE
+         ... SET</literal>.
+        </para>
+       </tip>
+      </listitem>
+     </varlistentry>
      <varlistentry>
       <term><replaceable class="parameter">lc_collate</replaceable></term>
       <listitem>
@@ -287,7 +308,7 @@ <title>Examples</title>
    To create a database <literal>music</literal> with a different locale:
 <programlisting>
 CREATE DATABASE music
-    LC_COLLATE 'sv_SE.utf8' LC_CTYPE 'sv_SE.utf8'
+    LOCALE 'sv_SE.utf8'
     TEMPLATE template0;
 </programlisting>
     In this example, the <literal>TEMPLATE template0</literal> clause is 
required if
@@ -300,7 +321,7 @@ <title>Examples</title>
    different character set encoding:
 <programlisting>
 CREATE DATABASE music2
-    LC_COLLATE 'sv_SE.iso885915' LC_CTYPE 'sv_SE.iso885915'
+    LOCALE 'sv_SE.iso885915'
     ENCODING LATIN9
     TEMPLATE template0;
 </programlisting>
diff --git a/src/backend/commands/dbcommands.c 
b/src/backend/commands/dbcommands.c
index 863f89f19d..fc1e1564a6 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -124,6 +124,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
        DefElem    *downer = NULL;
        DefElem    *dtemplate = NULL;
        DefElem    *dencoding = NULL;
+       DefElem    *dlocale = NULL;
        DefElem    *dcollate = NULL;
        DefElem    *dctype = NULL;
        DefElem    *distemplate = NULL;
@@ -184,6 +185,15 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
                                                 parser_errposition(pstate, 
defel->location)));
                        dencoding = defel;
                }
+               else if (strcmp(defel->defname, "locale") == 0)
+               {
+                       if (dlocale)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_SYNTAX_ERROR),
+                                                errmsg("conflicting or 
redundant options"),
+                                                parser_errposition(pstate, 
defel->location)));
+                       dlocale = defel;
+               }
                else if (strcmp(defel->defname, "lc_collate") == 0)
                {
                        if (dcollate)
@@ -244,6 +254,12 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
                                         parser_errposition(pstate, 
defel->location)));
        }
 
+       if (dlocale && (dcollate || dctype))
+               ereport(ERROR,
+                               (errcode(ERRCODE_SYNTAX_ERROR),
+                                errmsg("conflicting or redundant options"),
+                                errdetail("LOCALE cannot be specified together 
with LC_COLLATE or LC_CTYPE.")));
+
        if (downer && downer->arg)
                dbowner = defGetString(downer);
        if (dtemplate && dtemplate->arg)
@@ -276,6 +292,11 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
                                                 parser_errposition(pstate, 
dencoding->location)));
                }
        }
+       if (dlocale && dlocale->arg)
+       {
+               dbcollate = defGetString(dlocale);
+               dbctype = defGetString(dlocale);
+       }
        if (dcollate && dcollate->arg)
                dbcollate = defGetString(dcollate);
        if (dctype && dctype->arg)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bbf44a1820..8a31672247 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2812,15 +2812,23 @@ dumpDatabase(Archive *fout)
                appendPQExpBufferStr(creaQry, " ENCODING = ");
                appendStringLiteralAH(creaQry, encoding, fout);
        }
-       if (strlen(collate) > 0)
+       if (strlen(collate) > 0 && strcmp(collate, ctype) == 0)
        {
-               appendPQExpBufferStr(creaQry, " LC_COLLATE = ");
+               appendPQExpBufferStr(creaQry, " LOCALE = ");
                appendStringLiteralAH(creaQry, collate, fout);
        }
-       if (strlen(ctype) > 0)
+       else
        {
-               appendPQExpBufferStr(creaQry, " LC_CTYPE = ");
-               appendStringLiteralAH(creaQry, ctype, fout);
+               if (strlen(collate) > 0)
+               {
+                       appendPQExpBufferStr(creaQry, " LC_COLLATE = ");
+                       appendStringLiteralAH(creaQry, collate, fout);
+               }
+               if (strlen(ctype) > 0)
+               {
+                       appendPQExpBufferStr(creaQry, " LC_CTYPE = ");
+                       appendStringLiteralAH(creaQry, ctype, fout);
+               }
        }
 
        /*
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index c56bf00e4b..7cbccee103 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -1407,6 +1407,15 @@
                like => { pg_dumpall_dbprivs => 1, },
        },
 
+       "CREATE DATABASE dump_test2 LOCALE = 'C'" => {
+               create_order => 47,
+               create_sql   => "CREATE DATABASE dump_test2 LOCALE = 'C' 
TEMPLATE = template0;",
+               regexp       => qr/^
+                       \QCREATE DATABASE dump_test2 \E.*\QLOCALE = 'C';\E
+                       /xm,
+               like => { pg_dumpall_dbprivs => 1, },
+       },
+
        'CREATE EXTENSION ... plpgsql' => {
                regexp => qr/^
                        \QCREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA 
pg_catalog;\E

base-commit: 7961886580a594e519ca7ed1811b464206738be5
-- 
2.22.0

Reply via email to