From 510224af089b1b81b5c990283de88f9952db163f Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Wed, 26 May 2021 19:43:42 +0530
Subject: [PATCH v2] Check duplicate options and error out for CREATE COLLATION

CREATE COLLATION doesn't throw an error if duplicate options are
specified. Modify the option parsing loop in DefineCollation to
check for duplicate options and error out if found one.
---
 src/backend/commands/collationcmds.c  | 42 +++++++++++++++++++++++++++
 src/test/regress/expected/collate.out | 40 +++++++++++++++++++++++++
 src/test/regress/sql/collate.sql      | 19 ++++++++++++
 3 files changed, 101 insertions(+)

diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index ebb0994db3..4620069aba 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -88,17 +88,59 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 		if (strcmp(defel->defname, "from") == 0)
 			defelp = &fromEl;
 		else if (strcmp(defel->defname, "locale") == 0)
+		{
+			if (localeEl)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("option \"%s\" specified more than once", defel->defname),
+						 parser_errposition(pstate, defel->location)));
 			defelp = &localeEl;
+		}
 		else if (strcmp(defel->defname, "lc_collate") == 0)
+		{
+			if (lccollateEl)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("option \"%s\" specified more than once", defel->defname),
+						 parser_errposition(pstate, defel->location)));
 			defelp = &lccollateEl;
+		}
 		else if (strcmp(defel->defname, "lc_ctype") == 0)
+		{
+			if (lcctypeEl)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("option \"%s\" specified more than once", defel->defname),
+						 parser_errposition(pstate, defel->location)));
 			defelp = &lcctypeEl;
+		}
 		else if (strcmp(defel->defname, "provider") == 0)
+		{
+			if (providerEl)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("option \"%s\" specified more than once", defel->defname),
+						 parser_errposition(pstate, defel->location)));
 			defelp = &providerEl;
+		}
 		else if (strcmp(defel->defname, "deterministic") == 0)
+		{
+			if (deterministicEl)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("option \"%s\" specified more than once", defel->defname),
+						 parser_errposition(pstate, defel->location)));
 			defelp = &deterministicEl;
+		}
 		else if (strcmp(defel->defname, "version") == 0)
+		{
+			if (versionEl)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("option \"%s\" specified more than once", defel->defname),
+						 parser_errposition(pstate, defel->location)));
 			defelp = &versionEl;
+		}
 		else
 		{
 			ereport(ERROR,
diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out
index 0b60b55514..edced86af2 100644
--- a/src/test/regress/expected/collate.out
+++ b/src/test/regress/expected/collate.out
@@ -701,6 +701,46 @@ View definition:
  SELECT ss.c1 + 1 AS c1p
    FROM ( SELECT 4 AS c1) ss;
 
+-- Check duplicate options in CREATE COLLATION command and throw error
+-- LC_COLLATE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LC_COLLATE = "NONSENSE", LC_CTYPE = "POSIX");
+ERROR:  option "lc_collate" specified more than once
+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:  option "lc_ctype" specified more than once
+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:  option "provider" specified more than once
+LINE 1: CREATE COLLATION coll_dup_chk (PROVIDER = icu, PROVIDER = NO...
+                                                       ^
+-- LOCALE
+CREATE COLLATION case_sensitive (LOCALE = '', LOCALE = "NONSENSE");
+ERROR:  option "locale" specified more than once
+LINE 1: CREATE COLLATION case_sensitive (LOCALE = '', LOCALE = "NONS...
+                                                      ^
+-- DETERMINISTIC
+CREATE COLLATION coll_dup_chk (DETERMINISTIC = TRUE, DETERMINISTIC = NONSENSE, LOCALE = '');
+ERROR:  option "deterministic" specified more than once
+LINE 1: ...ATE COLLATION coll_dup_chk (DETERMINISTIC = TRUE, DETERMINIS...
+                                                             ^
+-- VERSION
+CREATE COLLATION coll_dup_chk (VERSION = '1', VERSION = "NONSENSE", LOCALE = '');
+ERROR:  option "version" specified more than once
+LINE 1: CREATE COLLATION coll_dup_chk (VERSION = '1', VERSION = "NON...
+                                                      ^
+-- LOCALE conflicting with LC_COLLATE and LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LC_CTYPE = "POSIX", LOCALE = '');
+ERROR:  conflicting or redundant options
+-- LOCALE conflicting with LC_COLLATE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LOCALE = '');
+ERROR:  conflicting or redundant options
+-- LOCALE conflicting with LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_CTYPE = "POSIX", LOCALE = '');
+ERROR:  conflicting or redundant 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 30f811ba89..d691fae9d4 100644
--- a/src/test/regress/sql/collate.sql
+++ b/src/test/regress/sql/collate.sql
@@ -272,6 +272,25 @@ SELECT c1+1 AS c1p FROM
   (SELECT ('4' COLLATE "C")::INT AS c1) ss;
 \d+ collate_on_int
 
+-- Check duplicate options in CREATE COLLATION command and throw error
+-- 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 conflicting with LC_COLLATE and LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LC_CTYPE = "POSIX", LOCALE = '');
+-- LOCALE conflicting with LC_COLLATE
+CREATE COLLATION coll_dup_chk (LC_COLLATE = "POSIX", LOCALE = '');
+-- LOCALE conflicting with LC_CTYPE
+CREATE COLLATION coll_dup_chk (LC_CTYPE = "POSIX", LOCALE = '');
 
 --
 -- Clean up.  Many of these table names will be re-used if the user is
-- 
2.25.1

