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