Improve error checking of CREATE COLLATION options.

Check for conflicting or redundant options, as we do for most other
commands. Specifying any option more than once is at best redundant,
and quite likely indicates a bug in the user's code.

While at it, improve the error for conflicting locale options by
adding detail text (the same as for CREATE DATABASE).

Bharath Rupireddy, reviewed by Vignesh C. Some additional hacking by
me.

Discussion: https://postgr.es/m/CALj2ACWtL6fTLdyF4R_YkPtf1YEDb6FUoD5DGAki3rpD+sWqiA@mail.gmail.com

diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index ebb0994db32af41ee03d80b2173ffae49e6f4204..5a2ba56cecacccb256f77e90c817dbd317a01ac5 100644 (file)
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -108,15 +108,22 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
                                         parser_errposition(pstate, defel->location)));
                        break;
                }
-
+               if (*defelp != NULL)
+                       errorConflictingDefElem(defel, pstate);
                *defelp = defel;
        }
 
-       if ((localeEl && (lccollateEl || lcctypeEl))
-               || (fromEl && list_length(parameters) != 1))
+       if (localeEl && (lccollateEl || lcctypeEl))
+               ereport(ERROR,
+                               errcode(ERRCODE_SYNTAX_ERROR),
+                               errmsg("conflicting or redundant options"),
+                               errdetail("LOCALE cannot be specified together with LC_COLLATE or LC_CTYPE."));
+
+       if (fromEl && list_length(parameters) != 1)
                ereport(ERROR,
-                               (errcode(ERRCODE_SYNTAX_ERROR),
-                                errmsg("conflicting or redundant options")));
+                               errcode(ERRCODE_SYNTAX_ERROR),
+                               errmsg("conflicting or redundant options"),
+                               errdetail("FROM cannot be specified together with any other options."));
 
        if (fromEl)
        {

diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out
index 0b60b55514747e1a1bf8aee4fc320d8dfcd0f362..246832575c432813e90925298158585cfc5d3d76 100644 (file)
--- a/src/test/regress/expected/collate.out
+++ b/src/test/regress/expected/collate.out
@@ -701,6 +701,53 @@ View definition:
  SELECT ss.c1 + 1 AS c1p
    FROM ( SELECT 4 AS c1) ss;
 
+-- Check conflicting or redundant options in CREATE COLLATION
+-- LC_COLLATE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LC_COLLATE = "NONSENSE", LC_CTYPE = "POSIX");
+ERROR:  conflicting or redundant options
+LINE 1: ...ATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LC_COLLATE...
+                                                             ^
+-- LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_CTYPE = "POSIX", LC_CTYPE = "NONSENSE", LC_COLLATE = "POSIX");
+ERROR:  conflicting or redundant options
+LINE 1: ...REATE COLLATION coll_dup_chk (LC_CTYPE = "POSIX", LC_CTYPE =...
+                                                             ^
+-- PROVIDER
+CREATE COLLATION coll_dup_chk (PROVIDER = icu, PROVIDER = NONSENSE, LC_COLLATE = "POSIX", LC_CTYPE = "POSIX");
+ERROR:  conflicting or redundant options
+LINE 1: CREATE COLLATION coll_dup_chk (PROVIDER = icu, PROVIDER = NO...
+                                                       ^
+-- LOCALE
+CREATE COLLATION case_sensitive (LOCALE = '', LOCALE = "NONSENSE");
+ERROR:  conflicting or redundant options
+LINE 1: CREATE COLLATION case_sensitive (LOCALE = '', LOCALE = "NONS...
+                                                      ^
+-- DETERMINISTIC
+CREATE COLLATION coll_dup_chk (DETERMINISTIC = TRUE, DETERMINISTIC = NONSENSE, LOCALE = '');
+ERROR:  conflicting or redundant options
+LINE 1: ...ATE COLLATION coll_dup_chk (DETERMINISTIC = TRUE, DETERMINIS...
+                                                             ^
+-- VERSION
+CREATE COLLATION coll_dup_chk (VERSION = '1', VERSION = "NONSENSE", LOCALE = '');
+ERROR:  conflicting or redundant options
+LINE 1: CREATE COLLATION coll_dup_chk (VERSION = '1', VERSION = "NON...
+                                                      ^
+-- LOCALE conflicts with LC_COLLATE and LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LC_CTYPE = "POSIX", LOCALE = '');
+ERROR:  conflicting or redundant options
+DETAIL:  LOCALE cannot be specified together with LC_COLLATE or LC_CTYPE.
+-- LOCALE conflicts with LC_COLLATE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LOCALE = '');
+ERROR:  conflicting or redundant options
+DETAIL:  LOCALE cannot be specified together with LC_COLLATE or LC_CTYPE.
+-- LOCALE conflicts with LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_CTYPE = "POSIX", LOCALE = '');
+ERROR:  conflicting or redundant options
+DETAIL:  LOCALE cannot be specified together with LC_COLLATE or LC_CTYPE.
+-- FROM conflicts with any other option
+CREATE COLLATION coll_dup_chk (FROM = "C", VERSION = "1");
+ERROR:  conflicting or redundant options
+DETAIL:  FROM cannot be specified together with any other options.
 --
 -- Clean up.  Many of these table names will be re-used if the user is
 -- trying to run any platform-specific collation tests later, so we

diff --git a/src/test/regress/sql/collate.sql b/src/test/regress/sql/collate.sql
index 30f811ba896937654a43421e55dfc025387ddec5..c3d40fc1959c04d0353587547f9c6576c0ace596 100644 (file)
--- a/src/test/regress/sql/collate.sql
+++ b/src/test/regress/sql/collate.sql
@@ -272,6 +272,27 @@ SELECT c1+1 AS c1p FROM
   (SELECT ('4' COLLATE "C")::INT AS c1) ss;
 \d+ collate_on_int
 
+-- Check conflicting or redundant options in CREATE COLLATION
+-- LC_COLLATE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LC_COLLATE = "NONSENSE", LC_CTYPE = "POSIX");
+-- LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_CTYPE = "POSIX", LC_CTYPE = "NONSENSE", LC_COLLATE = "POSIX");
+-- PROVIDER
+CREATE COLLATION coll_dup_chk (PROVIDER = icu, PROVIDER = NONSENSE, LC_COLLATE = "POSIX", LC_CTYPE = "POSIX");
+-- LOCALE
+CREATE COLLATION case_sensitive (LOCALE = '', LOCALE = "NONSENSE");
+-- DETERMINISTIC
+CREATE COLLATION coll_dup_chk (DETERMINISTIC = TRUE, DETERMINISTIC = NONSENSE, LOCALE = '');
+-- VERSION
+CREATE COLLATION coll_dup_chk (VERSION = '1', VERSION = "NONSENSE", LOCALE = '');
+-- LOCALE conflicts with LC_COLLATE and LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LC_CTYPE = "POSIX", LOCALE = '');
+-- LOCALE conflicts with LC_COLLATE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LOCALE = '');
+-- LOCALE conflicts with LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_CTYPE = "POSIX", LOCALE = '');
+-- FROM conflicts with any other option
+CREATE COLLATION coll_dup_chk (FROM = "C", VERSION = "1");
 
 --
 -- Clean up.  Many of these table names will be re-used if the user is