Merge pull request #1948 from citusdata/feature/alter-index

Add support for Alter Index Commands, and Storage Parameters on tables and indexes.

src/backend/distributed/executor/multi_utility.c

@@ -139,6 +139,7 @@ static void ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree);
 static void ErrorIfUnsupportedIndexStmt(IndexStmt *createIndexStatement);
 static void ErrorIfUnsupportedDropIndexStmt(DropStmt *dropIndexStatement);
 static void ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement);
+static void ErrorIfUnsupportedAlterIndexStmt(AlterTableStmt *alterTableStatement);
 static void ErrorIfAlterDropsPartitionColumn(AlterTableStmt *alterTableStatement);
 static void ErrorIfUnsupportedSeqStmt(CreateSeqStmt *createSeqStmt);
 static void ErrorIfDistributedAlterSeqOwnedBy(AlterSeqStmt *alterSeqStmt);
@@ -155,6 +156,7 @@ static void ErrorIfUnsupportedForeignConstraint(Relation relation,
 static char * ExtractNewExtensionVersion(Node *parsetree);
 static void CreateLocalTable(RangeVar *relation, char *nodeName, int32 nodePort);
 static bool IsAlterTableRenameStmt(RenameStmt *renameStmt);
+static bool IsIndexRenameStmt(RenameStmt *renameStmt);
 static bool AlterInvolvesPartitionColumn(AlterTableStmt *alterTableStatement,
 										 AlterTableCmd *command);
 static void ExecuteDistributedDDLJob(DDLJob *ddlJob);
@@ -365,7 +367,8 @@ multi_ProcessUtility(PlannedStmt *pstmt,
 		if (IsA(parsetree, AlterTableStmt))
 		{
 			AlterTableStmt *alterTableStmt = (AlterTableStmt *) parsetree;
-			if (alterTableStmt->relkind == OBJECT_TABLE)
+			if (alterTableStmt->relkind == OBJECT_TABLE ||
+				alterTableStmt->relkind == OBJECT_INDEX)
 			{
 				ddlJobs = PlanAlterTableStmt(alterTableStmt, queryString);
 			}
@@ -1237,6 +1240,7 @@ PlanAlterTableStmt(AlterTableStmt *alterTableStatement, const char *alterTableCo
 	LOCKMODE lockmode = 0;
 	Oid leftRelationId = InvalidOid;
 	Oid rightRelationId = InvalidOid;
+	char leftRelationKind;
 	bool isDistributedRelation = false;
 	List *commandList = NIL;
 	ListCell *commandCell = NULL;
@@ -1254,13 +1258,38 @@ PlanAlterTableStmt(AlterTableStmt *alterTableStatement, const char *alterTableCo
 		return NIL;
 	}
 
+	/*
+	 * AlterTableStmt applies also to INDEX relations, and we have support for
+	 * SET/SET storage parameters in Citus, so we might have to check for
+	 * another relation here.
+	 */
+	leftRelationKind = get_rel_relkind(leftRelationId);
+	if (leftRelationKind == RELKIND_INDEX)
+	{
+		leftRelationId = IndexGetRelation(leftRelationId, false);
+	}
+
 	isDistributedRelation = IsDistributedTable(leftRelationId);
 	if (!isDistributedRelation)
 	{
 		return NIL;
 	}
 
-	ErrorIfUnsupportedAlterTableStmt(alterTableStatement);
+	/*
+	 * The PostgreSQL parser dispatches several commands into the node type
+	 * AlterTableStmt, from ALTER INDEX to ALTER SEQUENCE or ALTER VIEW. Here
+	 * we have a special implementation for ALTER INDEX, and a specific error
+	 * message in case of unsupported sub-command.
+	 */
+	if (leftRelationKind == RELKIND_INDEX)
+	{
+		ErrorIfUnsupportedAlterIndexStmt(alterTableStatement);
+	}
+	else
+	{
+		/* this function also accepts more than just RELKIND_RELATION... */
+		ErrorIfUnsupportedAlterTableStmt(alterTableStatement);
+	}
 
 	/*
 	 * We check if there is a ADD FOREIGN CONSTRAINT command in sub commands list.
@@ -1371,11 +1400,17 @@ PlanAlterTableStmt(AlterTableStmt *alterTableStatement, const char *alterTableCo
 static List *
 PlanRenameStmt(RenameStmt *renameStmt, const char *renameCommand)
 {
-	Oid relationId = InvalidOid;
+	Oid objectRelationId = InvalidOid; /* SQL Object OID */
+	Oid tableRelationId = InvalidOid; /* Relation OID, maybe not the same. */
 	bool isDistributedRelation = false;
 	DDLJob *ddlJob = NULL;
 
-	if (!IsAlterTableRenameStmt(renameStmt))
+	/*
+	 * We only support some of the PostgreSQL supported RENAME statements, and
+	 * our list include only renaming table and index (related) objects.
+	 */
+	if (!IsAlterTableRenameStmt(renameStmt) &&
+		!IsIndexRenameStmt(renameStmt))
 	{
 		return NIL;
 	}
@@ -1385,32 +1420,70 @@ PlanRenameStmt(RenameStmt *renameStmt, const char *renameCommand)
 	 * RenameRelation(), renameatt() and RenameConstraint(). However, since all
 	 * three statements have identical lock levels, we just use a single statement.
 	 */
-	relationId = RangeVarGetRelid(renameStmt->relation, AccessExclusiveLock,
+	objectRelationId = RangeVarGetRelid(renameStmt->relation,
-								  renameStmt->missing_ok);
+										AccessExclusiveLock,
+										renameStmt->missing_ok);
 
 	/*
 	 * If the table does not exist, don't do anything here to allow PostgreSQL
 	 * to throw the appropriate error or notice message later.
 	 */
-	if (!OidIsValid(relationId))
+	if (!OidIsValid(objectRelationId))
 	{
 		return NIL;
 	}
 
 	/* we have no planning to do unless the table is distributed */
-	isDistributedRelation = IsDistributedTable(relationId);
+	switch (renameStmt->renameType)
+	{
+		case OBJECT_TABLE:
+		case OBJECT_COLUMN:
+		case OBJECT_TABCONSTRAINT:
+		{
+			/* the target object is our tableRelationId. */
+			tableRelationId = objectRelationId;
+			break;
+		}
+
+		case OBJECT_INDEX:
+		{
+			/*
+			 * here, objRelationId points to the index relation entry, and we
+			 * are interested into the entry of the table on which the index is
+			 * defined.
+			 */
+			tableRelationId = IndexGetRelation(objectRelationId, false);
+			break;
+		}
+
+		default:
+
+			/*
+			 * Nodes that are not supported by Citus: we pass-through to the
+			 * main PostgreSQL executor. Any Citus-supported RenameStmt
+			 * renameType must appear above in the switch, explicitly.
+			 */
+			return NIL;
+	}
+
+	isDistributedRelation = IsDistributedTable(tableRelationId);
 	if (!isDistributedRelation)
 	{
 		return NIL;
 	}
 
+	/*
+	 * We might ERROR out on some commands, but only for Citus tables where
+	 * isDistributedRelation is true. That's why this test comes this late in
+	 * the function.
+	 */
 	ErrorIfUnsupportedRenameStmt(renameStmt);
 
 	ddlJob = palloc0(sizeof(DDLJob));
-	ddlJob->targetRelationId = relationId;
+	ddlJob->targetRelationId = tableRelationId;
 	ddlJob->concurrentIndexCmd = false;
 	ddlJob->commandString = renameCommand;
-	ddlJob->taskList = DDLTaskList(relationId, renameCommand);
+	ddlJob->taskList = DDLTaskList(tableRelationId, renameCommand);
 
 	return list_make1(ddlJob);
 }
@@ -1966,9 +2039,9 @@ ErrorIfUnsupportedDropIndexStmt(DropStmt *dropIndexStatement)
 
 
 /*
- * ErrorIfUnsupportedAlterTableStmt checks if the corresponding alter table statement
+ * ErrorIfUnsupportedAlterTableStmt checks if the corresponding alter table
- * is supported for distributed tables and errors out if it is not. Currently,
+ * statement is supported for distributed tables and errors out if it is not.
- * only the following commands are supported.
+ * Currently, only the following commands are supported.
  *
  * ALTER TABLE ADD|DROP COLUMN
  * ALTER TABLE ALTER COLUMN SET DATA TYPE
@@ -1976,6 +2049,8 @@ ErrorIfUnsupportedDropIndexStmt(DropStmt *dropIndexStatement)
  * ALTER TABLE SET|DROP DEFAULT
  * ALTER TABLE ADD|DROP CONSTRAINT
  * ALTER TABLE REPLICA IDENTITY
+ * ALTER TABLE SET ()
+ * ALTER TABLE RESET ()
  */
 static void
 ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement)
@@ -2125,14 +2200,71 @@ ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement)
 				break;
 			}
 
+			case AT_SetRelOptions:  /* SET (...) */
+			case AT_ResetRelOptions:    /* RESET (...) */
+			case AT_ReplaceRelOptions:  /* replace entire option list */
+			{
+				/* this command is supported by Citus */
+				break;
+			}
+
 			default:
 			{
-				ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				ereport(ERROR,
-								errmsg("alter table command is currently unsupported"),
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								errdetail("Only ADD|DROP COLUMN, SET|DROP NOT NULL, "
+						 errmsg("alter table command is currently unsupported"),
-										  "SET|DROP DEFAULT, ADD|DROP CONSTRAINT, "
+						 errdetail("Only ADD|DROP COLUMN, SET|DROP NOT NULL, "
-										  "ATTACH|DETACH PARTITION and TYPE subcommands "
+								   "SET|DROP DEFAULT, ADD|DROP CONSTRAINT, "
-										  "are supported.")));
+								   "SET (), RESET (), "
+								   "ATTACH|DETACH PARTITION and TYPE subcommands "
+								   "are supported.")));
+			}
+		}
+	}
+}
+
+
+/*
+ * ErrorIfUnsupportedAlterIndexStmt checks if the corresponding alter index
+ * statement is supported for distributed tables and errors out if it is not.
+ * Currently, only the following commands are supported.
+ *
+ * ALTER INDEX SET ()
+ * ALTER INDEX RESET ()
+ */
+static void
+ErrorIfUnsupportedAlterIndexStmt(AlterTableStmt *alterTableStatement)
+{
+	List *commandList = alterTableStatement->cmds;
+	ListCell *commandCell = NULL;
+
+	/* error out if any of the subcommands are unsupported */
+	foreach(commandCell, commandList)
+	{
+		AlterTableCmd *command = (AlterTableCmd *) lfirst(commandCell);
+		AlterTableType alterTableType = command->subtype;
+
+		switch (alterTableType)
+		{
+			case AT_SetRelOptions:  /* SET (...) */
+			case AT_ResetRelOptions:    /* RESET (...) */
+			case AT_ReplaceRelOptions:  /* replace entire option list */
+			{
+				/* this command is supported by Citus */
+				break;
+			}
+
+			/* unsupported create index statements */
+			case AT_SetTableSpace:
+			default:
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("alter index ... set tablespace ... "
+								"is currently unsupported"),
+						 errdetail("Only RENAME TO, SET (), and RESET () "
+								   "are supported.")));
+				return; /* keep compiler happy */
 			}
 		}
 	}
@@ -2733,14 +2865,14 @@ OptionsSpecifyOwnedBy(List *optionList, Oid *ownedByTableId)
  * ErrorIfDistributedRenameStmt errors out if the corresponding rename statement
  * operates on any part of a distributed table other than a column.
  *
- * Note: This function handles only those rename statements which operate on tables.
+ * Note: This function handles RenameStmt applied to relations handed by Citus.
+ * At the moment of writing this comment, it could be either tables or indexes.
  */
 static void
 ErrorIfUnsupportedRenameStmt(RenameStmt *renameStmt)
 {
-	Assert(IsAlterTableRenameStmt(renameStmt));
+	if (IsAlterTableRenameStmt(renameStmt) &&
-
+		renameStmt->renameType == OBJECT_TABCONSTRAINT)
-	if (renameStmt->renameType == OBJECT_TABCONSTRAINT)
 	{
 		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 						errmsg("renaming constraints belonging to distributed tables is "
@@ -2860,6 +2992,26 @@ IsAlterTableRenameStmt(RenameStmt *renameStmt)
 }
 
 
+/*
+ * IsIndexRenameStmt returns whether the passed-in RenameStmt is the following
+ * form:
+ *
+ *   - ALTER INDEX RENAME
+ */
+static bool
+IsIndexRenameStmt(RenameStmt *renameStmt)
+{
+	bool isIndexRenameStmt = false;
+
+	if (renameStmt->renameType == OBJECT_INDEX)
+	{
+		isIndexRenameStmt = true;
+	}
+
+	return isIndexRenameStmt;
+}
+
+
 /*
  * AlterInvolvesPartitionColumn checks if the given alter table command
  * involves relation's partition column.

src/backend/distributed/utils/citus_ruleutils.c

@@ -31,6 +31,7 @@
 #include "catalog/pg_extension.h"
 #include "catalog/pg_foreign_data_wrapper.h"
 #include "catalog/pg_index.h"
+#include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/extension.h"
 #include "distributed/citus_ruleutils.h"
@@ -45,6 +46,7 @@
 #include "nodes/parsenodes.h"
 #include "nodes/pg_list.h"
 #include "parser/parse_utilcmd.h"
+#include "parser/parser.h"
 #include "storage/lock.h"
 #include "utils/acl.h"
 #include "utils/array.h"
@@ -61,8 +63,11 @@
 
 
 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);
 
 /*
  * pg_get_extensiondef_string finds the foreign data wrapper that corresponds to
@@ -474,6 +479,20 @@ pg_get_tableschemadef_string(Oid tableRelationId, bool includeSequenceDefaults)
 		appendStringInfo(&buffer, " PARTITION BY %s ", partitioningInformation);
 	}
 #endif
+
+	/*
+	 * Add any reloptions (storage parameters) defined on the table in a WITH
+	 * clause.
+	 */
+	{
+		char *reloptions = flatten_reloptions(tableRelationId);
+		if (reloptions)
+		{
+			appendStringInfo(&buffer, " WITH (%s)", reloptions);
+			pfree(reloptions);
+		}
+	}
+
 	relation_close(relation, AccessShareLock);
 
 	return (buffer.data);
@@ -738,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)
 	{
@@ -1004,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)
@@ -1112,3 +1165,127 @@ pg_get_replica_identity_command(Oid tableRelationId)
 
 	return (buf->len > 0) ? buf->data : NULL;
 }
+
+
+/*
+ * Generate a C string representing a relation's reloptions, or NULL if none.
+ *
+ * This function comes from PostgreSQL source code in
+ * src/backend/utils/adt/ruleutils.c
+ */
+static char *
+flatten_reloptions(Oid relid)
+{
+	char *result = NULL;
+	HeapTuple tuple;
+	Datum reloptions;
+	bool isnull;
+
+	tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+	if (!HeapTupleIsValid(tuple))
+	{
+		elog(ERROR, "cache lookup failed for relation %u", relid);
+	}
+
+	reloptions = SysCacheGetAttr(RELOID, tuple,
+								 Anum_pg_class_reloptions, &isnull);
+	if (!isnull)
+	{
+		StringInfoData buf;
+		Datum *options;
+		int noptions;
+		int i;
+
+		initStringInfo(&buf);
+
+		deconstruct_array(DatumGetArrayTypeP(reloptions),
+						  TEXTOID, -1, false, 'i',
+						  &options, NULL, &noptions);
+
+		for (i = 0; i < noptions; i++)
+		{
+			char *option = TextDatumGetCString(options[i]);
+			char *name;
+			char *separator;
+			char *value;
+
+			/*
+			 * Each array element should have the form name=value.  If the "="
+			 * is missing for some reason, treat it like an empty value.
+			 */
+			name = option;
+			separator = strchr(option, '=');
+			if (separator)
+			{
+				*separator = '\0';
+				value = separator + 1;
+			}
+			else
+			{
+				value = "";
+			}
+
+			if (i > 0)
+			{
+				appendStringInfoString(&buf, ", ");
+			}
+			appendStringInfo(&buf, "%s=", quote_identifier(name));
+
+			/*
+			 * In general we need to quote the value; but to avoid unnecessary
+			 * clutter, do not quote if it is an identifier that would not
+			 * need quoting.  (We could also allow numbers, but that is a bit
+			 * trickier than it looks --- for example, are leading zeroes
+			 * significant?  We don't want to assume very much here about what
+			 * custom reloptions might mean.)
+			 */
+			if (quote_identifier(value) == value)
+			{
+				appendStringInfoString(&buf, value);
+			}
+			else
+			{
+				simple_quote_literal(&buf, value);
+			}
+
+			pfree(option);
+		}
+
+		result = buf.data;
+	}
+
+	ReleaseSysCache(tuple);
+
+	return result;
+}
+
+
+/*
+ * simple_quote_literal - Format a string as a SQL literal, append to buf
+ *
+ * This function comes from PostgreSQL source code in
+ * src/backend/utils/adt/ruleutils.c
+ */
+static void
+simple_quote_literal(StringInfo buf, const char *val)
+{
+	const char *valptr;
+
+	/*
+	 * We form the string literal according to the prevailing setting of
+	 * standard_conforming_strings; we never use E''. User is responsible for
+	 * making sure result is used correctly.
+	 */
+	appendStringInfoChar(buf, '\'');
+	for (valptr = val; *valptr; valptr++)
+	{
+		char ch = *valptr;
+
+		if (SQL_STR_DOUBLE(ch, !standard_conforming_strings))
+		{
+			appendStringInfoChar(buf, ch);
+		}
+		appendStringInfoChar(buf, ch);
+	}
+	appendStringInfoChar(buf, '\'');
+}

src/test/regress/expected/multi_reference_table.out

@@ -1385,7 +1385,7 @@ SELECT "Column", "Type", "Modifiers" FROM table_desc WHERE relid='reference_sche
 -- as we expect, setting WITH OIDS does not work for reference tables
 ALTER TABLE reference_schema.reference_table_ddl SET WITH OIDS;
 ERROR:  alter table command is currently unsupported
-DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, ATTACH|DETACH PARTITION and TYPE subcommands are supported.
+DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, SET (), RESET (), ATTACH|DETACH PARTITION and TYPE subcommands are supported.
 -- now test the renaming of the table, and back to the expected name
 ALTER TABLE reference_schema.reference_table_ddl RENAME TO reference_table_ddl_test;
 ALTER TABLE reference_schema.reference_table_ddl_test RENAME TO reference_table_ddl;

src/test/regress/input/multi_alter_table_statements.source

@@ -26,10 +26,18 @@ CREATE TABLE lineitem_alter (
 	l_shipinstruct char(25) not null,
 	l_shipmode char(10) not null,
 	l_comment varchar(44) not null
-	);
+	)
+  WITH ( fillfactor = 80 );
 SELECT master_create_distributed_table('lineitem_alter', 'l_orderkey', 'append');
 \copy lineitem_alter FROM '@abs_srcdir@/data/lineitem.1.data' with delimiter '|'
 
+-- verify that the storage options made it to the table definitions
+SELECT relname, reloptions FROM pg_class WHERE relname = 'lineitem_alter';
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relname;
+\c - - - :master_port
+
 -- Verify that we can add columns
 
 ALTER TABLE lineitem_alter ADD COLUMN float_column FLOAT;
@@ -372,9 +380,39 @@ ALTER TABLE lineitem_renamed RENAME TO lineitem_alter;
 
 -- show rename worked on one worker, too
 \c - - - :worker_1_port
-SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relname;
+SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' AND relname <> 'lineitem_alter_220009' /* failed copy trails */ ORDER BY relname;
 \c - - - :master_port
 
+-- verify that we can set and reset storage parameters
+ALTER TABLE lineitem_alter SET(fillfactor=40);
+SELECT relname, reloptions FROM pg_class WHERE relname = 'lineitem_alter';
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'lineitem_alter%' AND relname <> 'lineitem_alter_220009' /* failed copy trails */ ORDER BY relname;
+\c - - - :master_port
+
+ALTER TABLE lineitem_alter RESET(fillfactor);
+SELECT relname, reloptions FROM pg_class WHERE relname = 'lineitem_alter';
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'lineitem_alter%'  AND relname <> 'lineitem_alter_220009' /* failed copy trails */ ORDER BY relname;
+\c - - - :master_port
+
+-- verify that we can rename indexes on distributed tables
+CREATE INDEX temp_index_1 ON lineitem_alter(l_linenumber);
+ALTER INDEX temp_index_1 RENAME TO idx_lineitem_linenumber;
+
+-- verify rename is performed
+SELECT relname FROM pg_class WHERE relname = 'idx_lineitem_linenumber';
+
+-- show rename worked on one worker, too
+\c - - - :worker_1_port
+SELECT relname FROM pg_class WHERE relname LIKE 'idx_lineitem_linenumber%' ORDER BY relname;
+\c - - - :master_port
+
+-- now get rid of the index
+DROP INDEX idx_lineitem_linenumber;
+
 -- verify that we don't intercept DDL commands if propagation is turned off
 SET citus.enable_ddl_propagation to false;
 
@@ -502,3 +540,45 @@ END;
 \c - - - :worker_1_port
 SELECT relname FROM pg_class WHERE relname LIKE 'test_table_1%';
 \c - - - :master_port
+
+-- Test WITH options on a normal simple hash-distributed table
+CREATE TABLE hash_dist(id bigint primary key, f1 text) WITH (fillfactor=40);
+SELECT create_distributed_table('hash_dist','id');
+
+-- verify that the storage options made it to the table definitions
+SELECT relname, reloptions FROM pg_class WHERE relname = 'hash_dist';
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relkind = 'r' AND relname LIKE 'hash_dist%' ORDER BY relname;
+\c - - - :master_port
+
+-- verify that we can set and reset index storage parameters
+ALTER INDEX hash_dist_pkey SET(fillfactor=40);
+SELECT relname, reloptions FROM pg_class WHERE relname = 'hash_dist_pkey';
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'hash_dist_pkey%' ORDER BY relname;
+\c - - - :master_port
+
+ALTER INDEX hash_dist_pkey RESET(fillfactor);
+SELECT relname, reloptions FROM pg_class WHERE relname = 'hash_dist_pkey';
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'hash_dist_pkey%' ORDER BY relname;
+\c - - - :master_port
+
+-- 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

@@ -22,7 +22,8 @@ CREATE TABLE lineitem_alter (
 	l_shipinstruct char(25) not null,
 	l_shipmode char(10) not null,
 	l_comment varchar(44) not null
-	);
+	)
+  WITH ( fillfactor = 80 );
 SELECT master_create_distributed_table('lineitem_alter', 'l_orderkey', 'append');
  master_create_distributed_table 
 ---------------------------------
@@ -30,6 +31,24 @@ SELECT master_create_distributed_table('lineitem_alter', 'l_orderkey', 'append')
 (1 row)
 
 \copy lineitem_alter FROM '@abs_srcdir@/data/lineitem.1.data' with delimiter '|'
+-- verify that the storage options made it to the table definitions
+SELECT relname, reloptions FROM pg_class WHERE relname = 'lineitem_alter';
+    relname     |   reloptions    
+----------------+-----------------
+ lineitem_alter | {fillfactor=80}
+(1 row)
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relname;
+        relname        |   reloptions    
+-----------------------+-----------------
+ lineitem_alter_220000 | {fillfactor=80}
+ lineitem_alter_220001 | {fillfactor=80}
+ lineitem_alter_220002 | {fillfactor=80}
+ lineitem_alter_220003 | {fillfactor=80}
+(4 rows)
+
+\c - - - :master_port
 -- Verify that we can add columns
 ALTER TABLE lineitem_alter ADD COLUMN float_column FLOAT;
 ALTER TABLE lineitem_alter ADD COLUMN date_column DATE;
@@ -328,7 +347,7 @@ SELECT "Column", "Type", "Modifiers" FROM table_desc WHERE relid='public.lineite
 ALTER TABLE lineitem_alter ADD COLUMN int_column3 INTEGER,
 	ALTER COLUMN int_column1 SET STATISTICS 10;
 ERROR:  alter table command is currently unsupported
-DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, ATTACH|DETACH PARTITION and TYPE subcommands are supported.
+DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, SET (), RESET (), ATTACH|DETACH PARTITION and TYPE subcommands are supported.
 ALTER TABLE lineitem_alter DROP COLUMN int_column1, DROP COLUMN int_column2;
 SELECT "Column", "Type", "Modifiers" FROM table_desc WHERE relid='public.lineitem_alter'::regclass;
      Column      |         Type          | Modifiers 
@@ -360,12 +379,12 @@ ERROR:  cannot execute ALTER TABLE command involving partition column
 -- Verify that we error out on unsupported statement types
 ALTER TABLE lineitem_alter ALTER COLUMN l_orderkey SET STATISTICS 100;
 ERROR:  alter table command is currently unsupported
-DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, ATTACH|DETACH PARTITION and TYPE subcommands are supported.
+DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, SET (), RESET (), ATTACH|DETACH PARTITION and TYPE subcommands are supported.
 ALTER TABLE lineitem_alter DROP CONSTRAINT IF EXISTS non_existent_contraint;
 NOTICE:  constraint "non_existent_contraint" of relation "lineitem_alter" does not exist, skipping
 ALTER TABLE lineitem_alter SET WITHOUT OIDS;
 ERROR:  alter table command is currently unsupported
-DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, ATTACH|DETACH PARTITION and TYPE subcommands are supported.
+DETAIL:  Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT, ADD|DROP CONSTRAINT, SET (), RESET (), ATTACH|DETACH PARTITION and TYPE subcommands are supported.
 -- Verify that we error out in case of postgres errors on supported statement
 -- types
 ALTER TABLE lineitem_alter ADD COLUMN new_column non_existent_type;
@@ -823,7 +842,7 @@ SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_renamed%'  ORDER BY re
 ALTER TABLE lineitem_renamed RENAME TO lineitem_alter;
 -- show rename worked on one worker, too
 \c - - - :worker_1_port
-SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relname;
+SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' AND relname <> 'lineitem_alter_220009' /* failed copy trails */ ORDER BY relname;
         relname        
 -----------------------
  lineitem_alter_220000
@@ -835,15 +854,105 @@ SELECT relname FROM pg_class WHERE relname LIKE 'lineitem_alter%' ORDER BY relna
  lineitem_alter_220006
  lineitem_alter_220007
  lineitem_alter_220008
- lineitem_alter_220009
  lineitem_alter_220010
  lineitem_alter_220011
  lineitem_alter_220012
  lineitem_alter_220013
  lineitem_alter_220014
-(15 rows)
+(14 rows)
 
 \c - - - :master_port
+-- verify that we can set and reset storage parameters
+ALTER TABLE lineitem_alter SET(fillfactor=40);
+SELECT relname, reloptions FROM pg_class WHERE relname = 'lineitem_alter';
+    relname     |   reloptions    
+----------------+-----------------
+ lineitem_alter | {fillfactor=40}
+(1 row)
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'lineitem_alter%' AND relname <> 'lineitem_alter_220009' /* failed copy trails */ ORDER BY relname;
+        relname        |   reloptions    
+-----------------------+-----------------
+ lineitem_alter_220000 | {fillfactor=40}
+ lineitem_alter_220001 | {fillfactor=40}
+ lineitem_alter_220002 | {fillfactor=40}
+ lineitem_alter_220003 | {fillfactor=40}
+ lineitem_alter_220004 | {fillfactor=40}
+ lineitem_alter_220005 | {fillfactor=40}
+ lineitem_alter_220006 | {fillfactor=40}
+ lineitem_alter_220007 | {fillfactor=40}
+ lineitem_alter_220008 | {fillfactor=40}
+ lineitem_alter_220010 | {fillfactor=40}
+ lineitem_alter_220011 | {fillfactor=40}
+ lineitem_alter_220012 | {fillfactor=40}
+ lineitem_alter_220013 | {fillfactor=40}
+ lineitem_alter_220014 | {fillfactor=40}
+(14 rows)
+
+\c - - - :master_port
+ALTER TABLE lineitem_alter RESET(fillfactor);
+SELECT relname, reloptions FROM pg_class WHERE relname = 'lineitem_alter';
+    relname     | reloptions 
+----------------+------------
+ lineitem_alter | 
+(1 row)
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'lineitem_alter%'  AND relname <> 'lineitem_alter_220009' /* failed copy trails */ ORDER BY relname;
+        relname        | reloptions 
+-----------------------+------------
+ lineitem_alter_220000 | 
+ lineitem_alter_220001 | 
+ lineitem_alter_220002 | 
+ lineitem_alter_220003 | 
+ lineitem_alter_220004 | 
+ lineitem_alter_220005 | 
+ lineitem_alter_220006 | 
+ lineitem_alter_220007 | 
+ lineitem_alter_220008 | 
+ lineitem_alter_220010 | 
+ lineitem_alter_220011 | 
+ lineitem_alter_220012 | 
+ lineitem_alter_220013 | 
+ lineitem_alter_220014 | 
+(14 rows)
+
+\c - - - :master_port
+-- verify that we can rename indexes on distributed tables
+CREATE INDEX temp_index_1 ON lineitem_alter(l_linenumber);
+ALTER INDEX temp_index_1 RENAME TO idx_lineitem_linenumber;
+-- verify rename is performed
+SELECT relname FROM pg_class WHERE relname = 'idx_lineitem_linenumber';
+         relname         
+-------------------------
+ idx_lineitem_linenumber
+(1 row)
+
+-- show rename worked on one worker, too
+\c - - - :worker_1_port
+SELECT relname FROM pg_class WHERE relname LIKE 'idx_lineitem_linenumber%' ORDER BY relname;
+            relname             
+--------------------------------
+ idx_lineitem_linenumber_220000
+ idx_lineitem_linenumber_220001
+ idx_lineitem_linenumber_220002
+ idx_lineitem_linenumber_220003
+ idx_lineitem_linenumber_220004
+ idx_lineitem_linenumber_220005
+ idx_lineitem_linenumber_220006
+ idx_lineitem_linenumber_220007
+ idx_lineitem_linenumber_220008
+ idx_lineitem_linenumber_220010
+ idx_lineitem_linenumber_220011
+ idx_lineitem_linenumber_220012
+ idx_lineitem_linenumber_220013
+ idx_lineitem_linenumber_220014
+(14 rows)
+
+\c - - - :master_port
+-- now get rid of the index
+DROP INDEX idx_lineitem_linenumber;
 -- verify that we don't intercept DDL commands if propagation is turned off
 SET citus.enable_ddl_propagation to false;
 -- table rename statement can be performed on the coordinator only now
@@ -1021,3 +1130,92 @@ SELECT relname FROM pg_class WHERE relname LIKE 'test_table_1%';
 (0 rows)
 
 \c - - - :master_port
+-- Test WITH options on a normal simple hash-distributed table
+CREATE TABLE hash_dist(id bigint primary key, f1 text) WITH (fillfactor=40);
+SELECT create_distributed_table('hash_dist','id');
+ create_distributed_table 
+--------------------------
+ 
+(1 row)
+
+-- verify that the storage options made it to the table definitions
+SELECT relname, reloptions FROM pg_class WHERE relname = 'hash_dist';
+  relname  |   reloptions    
+-----------+-----------------
+ hash_dist | {fillfactor=40}
+(1 row)
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relkind = 'r' AND relname LIKE 'hash_dist%' ORDER BY relname;
+     relname      |   reloptions    
+------------------+-----------------
+ hash_dist_220033 | {fillfactor=40}
+ hash_dist_220034 | {fillfactor=40}
+ hash_dist_220035 | {fillfactor=40}
+ hash_dist_220036 | {fillfactor=40}
+(4 rows)
+
+\c - - - :master_port
+-- verify that we can set and reset index storage parameters
+ALTER INDEX hash_dist_pkey SET(fillfactor=40);
+SELECT relname, reloptions FROM pg_class WHERE relname = 'hash_dist_pkey';
+    relname     |   reloptions    
+----------------+-----------------
+ hash_dist_pkey | {fillfactor=40}
+(1 row)
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'hash_dist_pkey%' ORDER BY relname;
+        relname        |   reloptions    
+-----------------------+-----------------
+ hash_dist_pkey_220033 | {fillfactor=40}
+ hash_dist_pkey_220034 | {fillfactor=40}
+ hash_dist_pkey_220035 | {fillfactor=40}
+ hash_dist_pkey_220036 | {fillfactor=40}
+(4 rows)
+
+\c - - - :master_port
+ALTER INDEX hash_dist_pkey RESET(fillfactor);
+SELECT relname, reloptions FROM pg_class WHERE relname = 'hash_dist_pkey';
+    relname     | reloptions 
+----------------+------------
+ hash_dist_pkey | 
+(1 row)
+
+\c - - - :worker_1_port
+SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'hash_dist_pkey%' ORDER BY relname;
+        relname        | reloptions 
+-----------------------+------------
+ hash_dist_pkey_220033 | 
+ hash_dist_pkey_220034 | 
+ hash_dist_pkey_220035 | 
+ hash_dist_pkey_220036 | 
+(4 rows)
+
+\c - - - :master_port
+-- verify error message on ALTER INDEX, SET TABLESPACE is unsupported
+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;