Add CREATE DATABASE LOCALE option

Started by Peter Eisentrautover 6 years ago11 messages
#1Peter Eisentraut
peter.eisentraut@2ndquadrant.com
1 attachment(s)

I propose this patch to add a LOCALE option to CREATE DATABASE. This
sets both LC_COLLATE and LC_CTYPE with one option. Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate and
lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-Add-CREATE-DATABASE-LOCALE-option.patchtext/plain; charset=UTF-8; name=0001-Add-CREATE-DATABASE-LOCALE-option.patch; x-mac-creator=0; x-mac-type=0Download
From 5f64d10944b272a5b636d7b0033a0a6a3d28998b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 4 Jun 2019 14:45:00 +0200
Subject: [PATCH] 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.
---
 doc/src/sgml/ref/create_database.sgml | 15 +++++++++++++--
 src/backend/commands/dbcommands.c     | 20 ++++++++++++++++++++
 src/bin/pg_dump/pg_dump.c             | 23 +++++++++++++++++------
 3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/doc/src/sgml/ref/create_database.sgml b/doc/src/sgml/ref/create_database.sgml
index b2c9e241c2..c5b7af5cf7 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,16 @@ <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>
+      </listitem>
+     </varlistentry>
      <varlistentry>
       <term><replaceable class="parameter">lc_collate</replaceable></term>
       <listitem>
@@ -287,7 +298,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 +311,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 15207bf75a..ac275f7e64 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,11 @@ 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")));
+
 	if (downer && downer->arg)
 		dbowner = defGetString(downer);
 	if (dtemplate && dtemplate->arg)
@@ -276,6 +291,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 9f59cc74ee..ba34677787 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2812,15 +2812,26 @@ dumpDatabase(Archive *fout)
 		appendPQExpBufferStr(creaQry, " ENCODING = ");
 		appendStringLiteralAH(creaQry, encoding, fout);
 	}
-	if (strlen(collate) > 0)
+	if (strcmp(collate, ctype) == 0)
 	{
-		appendPQExpBufferStr(creaQry, " LC_COLLATE = ");
-		appendStringLiteralAH(creaQry, collate, fout);
+		if (strlen(collate) > 0)
+		{
+			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);
+		}
 	}
 
 	/*
-- 
2.21.0

#2Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Peter Eisentraut (#1)
Re: Add CREATE DATABASE LOCALE option

On Wed, Jun 5, 2019 at 5:17 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

I propose this patch to add a LOCALE option to CREATE DATABASE. This
sets both LC_COLLATE and LC_CTYPE with one option. Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate and
lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

Cool... would be nice also add some test cases.

Regards,

--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

#3Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Fabrízio de Royes Mello (#2)
Re: Add CREATE DATABASE LOCALE option

On 2019-06-05 22:31, Fabrízio de Royes Mello wrote:

On Wed, Jun 5, 2019 at 5:17 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:

I propose this patch to add a LOCALE option to CREATE DATABASE.  This
sets both LC_COLLATE and LC_CTYPE with one option.  Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate and
lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

Cool... would be nice also add some test cases.

Right. Any suggestions where to put them?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Peter Eisentraut (#3)
Re: Add CREATE DATABASE LOCALE option

On Thu, Jun 6, 2019 at 6:38 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2019-06-05 22:31, Fabrízio de Royes Mello wrote:

On Wed, Jun 5, 2019 at 5:17 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:

I propose this patch to add a LOCALE option to CREATE DATABASE. This
sets both LC_COLLATE and LC_CTYPE with one option. Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate

and

lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

Cool... would be nice also add some test cases.

Right. Any suggestions where to put them?

Hmm... good question... I thought we already have some regression tests for
{CREATE|DROP} DATABASE but actually we don't... should we add a new one?

Att,

--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fabrízio de Royes Mello (#4)
Re: Add CREATE DATABASE LOCALE option

On 2019-Jun-06, Fabr�zio de Royes Mello wrote:

Cool... would be nice also add some test cases.

Right. Any suggestions where to put them?

Hmm... good question... I thought we already have some regression tests for
{CREATE|DROP} DATABASE but actually we don't... should we add a new one?

I think pg_dump/t/002_pg_dump.pl might be a good place. Not the easiest
program in the world to work with, admittedly.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#6Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Peter Eisentraut (#1)
Re: Add CREATE DATABASE LOCALE option

On 05/06/2019 23:17, Peter Eisentraut wrote:

I propose this patch to add a LOCALE option to CREATE DATABASE. This
sets both LC_COLLATE and LC_CTYPE with one option. Similar behavior is
already supported in initdb, CREATE COLLATION, and createdb.

With collation providers other than libc, having separate lc_collate and
lc_ctype settings is not necessarily applicable, so this is also
preparation for such future functionality.

One objection is that the proposed LOCALE option would only affect
LC_COLLATE and LC_CTYPE. What about lc_messages, lc_monetary, lc_numeric
and lc_time? initdb's --locale option sets those, too. Should CREATE
DATABASE LOCALE set those as well?

On the whole, +1 on adding the option. In practice, you always want to
set LC_COLLATE and LC_CTYPE to the same value, so we should make that
easy. But let's consider those other variables too, at least we've got
to document it carefully.

PS. There was some discussion on doing this when the LC_COLLATE and
LC_CTYPE options were added:
/messages/by-id/491862F7.1060501@enterprisedb.com.
My reading of that is that there was no strong consensus, so we just let
it be.

- Heikki

#7Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Alvaro Herrera (#5)
1 attachment(s)
Re: Add CREATE DATABASE LOCALE option

On 2019-06-06 21:52, Alvaro Herrera wrote:

On 2019-Jun-06, Fabrízio de Royes Mello wrote:

Cool... would be nice also add some test cases.

Right. Any suggestions where to put them?

Hmm... good question... I thought we already have some regression tests for
{CREATE|DROP} DATABASE but actually we don't... should we add a new one?

I think pg_dump/t/002_pg_dump.pl might be a good place. Not the easiest
program in the world to work with, admittedly.

Updated patch with test and expanded documentation.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

v2-0001-Add-CREATE-DATABASE-LOCALE-option.patchtext/plain; charset=UTF-8; name=v2-0001-Add-CREATE-DATABASE-LOCALE-option.patch; x-mac-creator=0; x-mac-type=0Download
From 7ba4151235a32ebfd11bd80a5a01d9ef347d2c11 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sun, 23 Jun 2019 20:09:00 +0200
Subject: [PATCH v2] 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     | 20 ++++++++++++++++++++
 src/bin/pg_dump/pg_dump.c             | 23 +++++++++++++++++------
 src/bin/pg_dump/t/002_pg_dump.pl      |  9 +++++++++
 4 files changed, 69 insertions(+), 8 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 15207bf75a..ac275f7e64 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,11 @@ 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")));
+
 	if (downer && downer->arg)
 		dbowner = defGetString(downer);
 	if (dtemplate && dtemplate->arg)
@@ -276,6 +291,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 8909a45d61..5cad9ed796 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2812,15 +2812,26 @@ dumpDatabase(Archive *fout)
 		appendPQExpBufferStr(creaQry, " ENCODING = ");
 		appendStringLiteralAH(creaQry, encoding, fout);
 	}
-	if (strlen(collate) > 0)
+	if (strcmp(collate, ctype) == 0)
 	{
-		appendPQExpBufferStr(creaQry, " LC_COLLATE = ");
-		appendStringLiteralAH(creaQry, collate, fout);
+		if (strlen(collate) > 0)
+		{
+			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
-- 
2.22.0

#8Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Peter Eisentraut (#7)
Re: Add CREATE DATABASE LOCALE option

Hello Peter,

I think pg_dump/t/002_pg_dump.pl might be a good place. Not the easiest
program in the world to work with, admittedly.

Updated patch with test and expanded documentation.

Patch v2 applies cleanly, compiles, make check-world ok with tap enabled.
Doc gen ok.

The addition looks reasonable.

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.

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).

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

--
Fabien.

#9Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Fabien COELHO (#8)
1 attachment(s)
Re: Add CREATE DATABASE LOCALE option

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

Attachments:

v3-0001-Add-CREATE-DATABASE-LOCALE-option.patchtext/plain; charset=UTF-8; name=v3-0001-Add-CREATE-DATABASE-LOCALE-option.patch; x-mac-creator=0; x-mac-type=0Download
From df26107a550b9e1d0933a04dc31f43c43a17b0f7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@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

#10Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Peter Eisentraut (#9)
Re: Add CREATE DATABASE LOCALE option

Hello Peter,

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.

We will drag it anyway because LOCALE is just a shortcut for the other two
LC_* when they have the same value.

How about this patch?

It applies cleanly, compiles, global & pg_dump make check ok, doc gen ok.

I'm still unconvinced of the interest of breaking backward compatibility,
but this is no big deal.

I do not like much calling strlen() to check whether a string is empty,
but this is pre-existing.

I switched the patch to READY.

--
Fabien.

#11Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Fabien COELHO (#10)
Re: Add CREATE DATABASE LOCALE option

On 2019-07-23 00:18, Fabien COELHO wrote:

It applies cleanly, compiles, global & pg_dump make check ok, doc gen ok.

I'm still unconvinced of the interest of breaking backward compatibility,
but this is no big deal.

I do not like much calling strlen() to check whether a string is empty,
but this is pre-existing.

I switched the patch to READY.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services