Fix CREATE INDEX with storage options on distributed tables.

By sharing the implementation of the function AppendOptionListToString on
three call sites, we would expand an extra OPTIONS keyword in a create index
statement, and omit other bits of the specific syntax here.

This patch introduces an AppendStorageParametersToString() function that is
very similar to AppendOptionListToString() but handles WITH(a="foo",...)
syntax that is used in reloptions (aka Storage Parameters).

Fixes #1747.

src/backend/distributed/utils/citus_ruleutils.c

@@ -63,6 +63,8 @@
 
 
 static void AppendOptionListToString(StringInfo stringData, List *options);
+static void AppendStorageParametersToString(StringInfo stringBuffer,
+											List *optionList);
 static const char * convert_aclright_to_string(int aclright);
 static void simple_quote_literal(StringInfo buf, const char *val);
 static char * flatten_reloptions(Oid relid);
@@ -755,11 +757,7 @@ deparse_shard_index_statement(IndexStmt *origStmt, Oid distrelid, int64 shardid,
 
 	appendStringInfoString(buffer, ") ");
 
-	if (indexStmt->options != NIL)
+	AppendStorageParametersToString(buffer, indexStmt->options);
-	{
-		appendStringInfoString(buffer, "WITH ");
-		AppendOptionListToString(buffer, indexStmt->options);
-	}
 
 	if (indexStmt->whereClause != NULL)
 	{
@@ -1021,6 +1019,44 @@ AppendOptionListToString(StringInfo stringBuffer, List *optionList)
 }
 
 
+/*
+ * AppendStorageParametersToString converts the storage parameter list to its
+ * textual format, and appends this text to the given string buffer.
+ */
+static void
+AppendStorageParametersToString(StringInfo stringBuffer, List *optionList)
+{
+	ListCell *optionCell = NULL;
+	bool firstOptionPrinted = false;
+
+	if (optionList == NIL)
+	{
+		return;
+	}
+
+	appendStringInfo(stringBuffer, " WITH (");
+
+	foreach(optionCell, optionList)
+	{
+		DefElem *option = (DefElem *) lfirst(optionCell);
+		char *optionName = option->defname;
+		char *optionValue = defGetString(option);
+
+		if (firstOptionPrinted)
+		{
+			appendStringInfo(stringBuffer, ", ");
+		}
+		firstOptionPrinted = true;
+
+		appendStringInfo(stringBuffer, "%s = %s ",
+						 quote_identifier(optionName),
+						 quote_literal_cstr(optionValue));
+	}
+
+	appendStringInfo(stringBuffer, ")");
+}
+
+
 /* copy of postgresql's function, which is static as well */
 static const char *
 convert_aclright_to_string(int aclright)

src/test/regress/input/multi_alter_table_statements.source

@@ -569,3 +569,16 @@ SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'hash_dist_pkey%' OR
 
 -- verify error message on ALTER INDEX, SET TABLESPACE is unsupported
 ALTER INDEX hash_dist_pkey SET TABLESPACE foo;
+
+-- verify that we can add indexes with new storage options
+CREATE UNIQUE INDEX another_index ON hash_dist(id) WITH (fillfactor=50);
+
+-- show the index and its storage options on coordinator, then workers
+SELECT relname, reloptions FROM pg_class WHERE relname = 'another_index';
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'another_index%' ORDER BY relname;
+\c - - - :master_port
+
+-- get rid of the index
+DROP INDEX another_index;

src/test/regress/output/multi_alter_table_statements.source

@@ -1197,3 +1197,25 @@ SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'hash_dist_pkey%' OR
 ALTER INDEX hash_dist_pkey SET TABLESPACE foo;
 ERROR:  alter index ... set tablespace ... is currently unsupported
 DETAIL:  Only RENAME TO, SET (), and RESET () are supported.
+-- verify that we can add indexes with new storage options
+CREATE UNIQUE INDEX another_index ON hash_dist(id) WITH (fillfactor=50);
+-- show the index and its storage options on coordinator, then workers
+SELECT relname, reloptions FROM pg_class WHERE relname = 'another_index';
+    relname    |   reloptions    
+---------------+-----------------
+ another_index | {fillfactor=50}
+(1 row)
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'another_index%' ORDER BY relname;
+       relname        |   reloptions    
+----------------------+-----------------
+ another_index_220033 | {fillfactor=50}
+ another_index_220034 | {fillfactor=50}
+ another_index_220035 | {fillfactor=50}
+ another_index_220036 | {fillfactor=50}
+(4 rows)
+
+\c - - - :master_port
+-- get rid of the index
+DROP INDEX another_index;