Merge pull request #5115 from citusdata/velioglu/partition_fixes

Support for CREATE INDEX ONLY and ALTER INDEX ATTACH PARTITION

src/backend/distributed/commands/index.c

@@ -810,6 +810,7 @@ ErrorIfUnsupportedAlterIndexStmt(AlterTableStmt *alterTableStatement)
 			case AT_ResetRelOptions:    /* RESET (...) */
 			case AT_ReplaceRelOptions:  /* replace entire option list */
 			case AT_SetStatistics:  /* SET STATISTICS */
+			case AT_AttachPartition: /* ATTACH PARTITION */
 			{
 				/* this command is supported by Citus */
 				break;
@@ -823,7 +824,7 @@ ErrorIfUnsupportedAlterIndexStmt(AlterTableStmt *alterTableStatement)
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 						 errmsg("alter index ... set tablespace ... "
 								"is currently unsupported"),
-						 errdetail("Only RENAME TO, SET (), RESET () "
+						 errdetail("Only RENAME TO, SET (), RESET (), ATTACH PARTITION "
 								   "and SET STATISTICS are supported.")));
 				return; /* keep compiler happy */
 			}

src/backend/distributed/commands/table.c

@@ -533,9 +533,13 @@ PreprocessAlterTableStmt(Node *node, const char *alterTableCommand,
 	 * 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.
+	 *
+	 * ALTER INDEX ATTACH PARTITION also applies to INDEX relation, so we might
+	 * check another relation for that option as well.
 	 */
 	char leftRelationKind = get_rel_relkind(leftRelationId);
-	if (leftRelationKind == RELKIND_INDEX)
+	if (leftRelationKind == RELKIND_INDEX ||
+		leftRelationKind == RELKIND_PARTITIONED_INDEX)
 	{
 		bool missingOk = false;
 		leftRelationId = IndexGetRelation(leftRelationId, missingOk);
@@ -598,7 +602,8 @@ PreprocessAlterTableStmt(Node *node, const char *alterTableCommand,
 	 * we have a special implementation for ALTER INDEX, and a specific error
 	 * message in case of unsupported sub-command.
 	 */
-	if (leftRelationKind == RELKIND_INDEX)
+	if (leftRelationKind == RELKIND_INDEX ||
+		leftRelationKind == RELKIND_PARTITIONED_INDEX)
 	{
 		ErrorIfUnsupportedAlterIndexStmt(alterTableStatement);
 	}
@@ -820,24 +825,46 @@ PreprocessAlterTableStmt(Node *node, const char *alterTableCommand,
 		else if (alterTableType == AT_AttachPartition)
 		{
 			PartitionCmd *partitionCommand = (PartitionCmd *) command->def;
+			Oid attachedRelationId = RangeVarGetRelid(partitionCommand->name, NoLock,
+													  false);
+			char attachedRelationKind = get_rel_relkind(attachedRelationId);
 
 			/*
-			 * We only support ALTER TABLE ATTACH PARTITION, if it is only subcommand of
+			 * We support ALTER INDEX ATTACH PARTITION and ALTER TABLE ATTACH PARTITION
-			 * ALTER TABLE. It was already checked in ErrorIfUnsupportedAlterTableStmt.
+			 * if it is only subcommand of ALTER TABLE command. Since the attached relation
+			 * type is index for ALTER INDEX ATTACH PARTITION, we need to use the relation
+			 * id this index is created for.
+			 *
+			 * Both were already checked in ErrorIfUnsupportedAlterIndexStmt and
+			 * ErrorIfUnsupportedAlterTableStmt.
 			 */
-			Assert(list_length(commandList) <= 1);
+			if (attachedRelationKind == RELKIND_INDEX)
+			{
+				bool missingOk = false;
+				rightRelationId = IndexGetRelation(attachedRelationId, missingOk);
 
-			rightRelationId = RangeVarGetRelid(partitionCommand->name, NoLock, false);
+				/*
+				 * Since left relation is checked above to make sure it is Citus table,
+				 * partition of that must be Citus table as well.
+				 */
+				Assert(IsCitusTable(rightRelationId));
+			}
+			else if (attachedRelationKind == RELKIND_RELATION)
+			{
+				Assert(list_length(commandList) <= 1);
 
 				/*
 				 * Do not generate tasks if relation is distributed and the partition
 				 * is not distributed. Because, we'll manually convert the partition into
 				 * distributed table and co-locate with its parent.
 				 */
-			if (!IsCitusTable(rightRelationId))
+				if (!IsCitusTable(attachedRelationId))
 				{
 					return NIL;
 				}
+
+				rightRelationId = attachedRelationId;
+			}
 		}
 		else if (alterTableType == AT_DetachPartition)
 		{
@@ -900,7 +927,7 @@ PreprocessAlterTableStmt(Node *node, const char *alterTableCommand,
 		}
 		else
 		{
-			/* if foreign key related, use specialized task list function ... */
+			/* if foreign key or attaching partition index related, use specialized task list function ... */
 			ddlJob->taskList = InterShardDDLTaskList(leftRelationId, rightRelationId,
 													 sqlForTaskList);
 		}
@@ -2606,11 +2633,11 @@ SetupExecutionModeForAlterTable(Oid relationId, AlterTableCmd *command)
 /*
  * InterShardDDLTaskList builds a list of tasks to execute a inter shard DDL command on a
  * shards of given list of distributed table. At the moment this function is used to run
- * foreign key and partitioning command on worker node.
+ * foreign key, partitioning and attaching partition index command on worker node.
  *
  * leftRelationId is the relation id of actual distributed table which given command is
- * applied. rightRelationId is the relation id of distributed table which given command
+ * applied. rightRelationId is the relation id of either index or distributed table which
- * refers to.
+ * given command refers to.
  */
 static List *
 InterShardDDLTaskList(Oid leftRelationId, Oid rightRelationId,

src/backend/distributed/deparser/citus_ruleutils.c

@@ -695,11 +695,12 @@ deparse_shard_index_statement(IndexStmt *origStmt, Oid distrelid, int64 shardid,
 	List *deparseContext = deparse_context_for(relationName, distrelid);
 	indexStmt = transformIndexStmt(distrelid, indexStmt, NULL);
 
-	appendStringInfo(buffer, "CREATE %s INDEX %s %s %s ON %s USING %s ",
+	appendStringInfo(buffer, "CREATE %s INDEX %s %s %s ON %s %s USING %s ",
 					 (indexStmt->unique ? "UNIQUE" : ""),
 					 (indexStmt->concurrent ? "CONCURRENTLY" : ""),
 					 (indexStmt->if_not_exists ? "IF NOT EXISTS" : ""),
 					 quote_identifier(indexName),
+					 (indexStmt->relation->inh ? "" : "ONLY"),
 					 quote_qualified_identifier(indexStmt->relation->schemaname,
 												relationName),
 					 indexStmt->accessMethod);

src/backend/distributed/relay/relay_event_utility.c

@@ -640,8 +640,8 @@ RelayEventExtendNames(Node *parseTree, char *schemaName, uint64 shardId)
 
 /*
  * RelayEventExtendNamesForInterShardCommands extends relation names in the given parse
- * tree for certain utility commands. The function more specifically extends table and
+ * tree for certain utility commands. The function more specifically extends table, index
- * constraint names in the parse tree by appending the given shardId; thereby
+ * and constraint names in the parse tree by appending the given shardId; thereby
  * avoiding name collisions in the database among sharded tables. This function
  * has the side effect of extending relation names in the parse tree.
  */

src/backend/distributed/worker/worker_shard_visibility.c

@@ -96,7 +96,7 @@ citus_table_is_visible(PG_FUNCTION_ARGS)
 		 * more meaningful debug message here.
 		 */
 		relKind = get_rel_relkind(relationId);
-		if (relKind == RELKIND_INDEX)
+		if (relKind == RELKIND_INDEX || relKind == RELKIND_PARTITIONED_INDEX)
 		{
 			ereport(DEBUG2, (errmsg("skipping index \"%s\" since it belongs to a shard",
 									get_rel_name(relationId))));
@@ -207,7 +207,7 @@ RelationIsAKnownShard(Oid shardRelationId)
 	 * as well.
 	 */
 	relKind = get_rel_relkind(shardRelationId);
-	if (relKind == RELKIND_INDEX)
+	if (relKind == RELKIND_INDEX || relKind == RELKIND_PARTITIONED_INDEX)
 	{
 		shardRelationId = IndexGetRelation(shardRelationId, false);
 	}

src/test/regress/expected/multi_index_statements.out

@@ -412,6 +412,107 @@ DEBUG:  the index name on the shards of the partition is too long, switching to
 CREATE INDEX f1
     ON test_index_creation1 USING btree
     (field1);
+-- should be able to create index only for parent on both
+-- coordinator and worker nodes
+CREATE INDEX parent_index
+    ON ONLY test_index_creation1 USING btree
+    (field1);
+-- show that we have parent index only on the parent table not on the partitions
+SELECT count(*) FROM pg_index WHERE indrelid::regclass::text = 'test_index_creation1' AND indexrelid::regclass::text = 'parent_index';
+ count
+---------------------------------------------------------------------
+     1
+(1 row)
+
+SELECT count(*) FROM pg_index WHERE indrelid::regclass::text LIKE 'test_index_creation1_p2020%' AND indexrelid::regclass::text LIKE 'parent_index%';
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+\c - - - :worker_1_port
+SET search_path TO multi_index_statements;
+-- show that we have parent index_* only on the parent shards not on the partition shards
+SELECT count(*) FROM pg_index WHERE indrelid::regclass::text LIKE 'test_index_creation1_%' AND indexrelid::regclass::text LIKE 'parent_index%';
+ count
+---------------------------------------------------------------------
+    16
+(1 row)
+
+SELECT count(*) FROM pg_index WHERE indrelid::regclass::text LIKE 'test_index_creation1_p2020%' AND indexrelid::regclass::text LIKE 'parent_index%';
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+\c - - - :master_port
+SET search_path TO multi_index_statements;
+-- attach child index of a partition to parent index of the partitioned table
+CREATE INDEX child_index
+    ON test_index_creation1_p2020_09_26 USING btree
+    (field1);
+ALTER INDEX parent_index ATTACH PARTITION child_index;
+-- show that child index inherits from parent index which means it is attached to it
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass::text = 'child_index' AND inhparent::regclass::text = 'parent_index';
+ count
+---------------------------------------------------------------------
+     1
+(1 row)
+
+\c - - - :worker_1_port
+SET search_path TO multi_index_statements;
+-- show that child indices of partition shards also inherit from parent indices of parent shards
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass::text LIKE 'child_index%' AND inhparent::regclass::text LIKE 'parent_index%';
+ count
+---------------------------------------------------------------------
+    16
+(1 row)
+
+\c - - - :master_port
+SET search_path TO multi_index_statements;
+-- verify error check for partitioned index
+ALTER INDEX parent_index SET TABLESPACE foo;
+ERROR:  alter index ... set tablespace ... is currently unsupported
+DETAIL:  Only RENAME TO, SET (), RESET (), ATTACH PARTITION and SET STATISTICS are supported.
+-- drop parent index and show that child index will also be dropped
+DROP INDEX parent_index;
+SELECT count(*) FROM pg_index where indexrelid::regclass::text = 'child_index';
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+-- show that having a foreign key to reference table causes sequential execution mode
+-- with ALTER INDEX ... ATTACH PARTITION
+CREATE TABLE index_creation_reference_table (id int primary key);
+SELECT create_reference_table('index_creation_reference_table');
+ create_reference_table
+---------------------------------------------------------------------
+
+(1 row)
+
+ALTER TABLE test_index_creation1 ADD CONSTRAINT foreign_key_to_ref_table
+    FOREIGN KEY (tenant_id)
+    REFERENCES index_creation_reference_table (id);
+CREATE INDEX parent_index ON ONLY test_index_creation1 USING btree (field1);
+CREATE INDEX child_index ON test_index_creation1_p2020_09_26 USING btree (field1);
+BEGIN;
+    show citus.multi_shard_modify_mode;
+ citus.multi_shard_modify_mode
+---------------------------------------------------------------------
+ parallel
+(1 row)
+
+    ALTER INDEX parent_index ATTACH PARTITION child_index;
+    show citus.multi_shard_modify_mode;
+ citus.multi_shard_modify_mode
+---------------------------------------------------------------------
+ sequential
+(1 row)
+
+ROLLBACK;
+DROP TABLE index_creation_reference_table CASCADE;
+NOTICE:  drop cascades to constraint foreign_key_to_ref_table on table test_index_creation1
 SELECT
 'CREATE TABLE distributed_table(' ||
 string_Agg('col' || x::text || ' int,', ' ') ||

src/test/regress/expected/multi_mx_partitioning.out

@@ -250,6 +250,76 @@ DROP TABLE partitioning_test;
 ERROR:  operation is not allowed on this node
 HINT:  Connect to the coordinator and run it again.
 \c - - - :master_port
+-- Make sure that creating index on only parent and child won't form any inheritance but it will
+-- be formed after attaching child index to parent index
+CREATE INDEX partitioning_test_2010_idx ON ONLY partitioning_test_2010 USING btree (id);
+-- Show that there is no inheritance relation for the index above
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass = 'partitioning_test_2010_idx'::regclass;
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+-- Now add the index only on parent and show that there won't be any inheritance for those indices
+CREATE INDEX partition_only_parent_index ON ONLY partitioning_test USING btree (id);
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass = 'partitioning_test_2010_idx'::regclass;
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+-- Now attach the child index to parent and show there is inheritance
+ALTER INDEX partition_only_parent_index ATTACH PARTITION partitioning_test_2010_idx;
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass = 'partitioning_test_2010_idx'::regclass;
+ count
+---------------------------------------------------------------------
+     1
+(1 row)
+
+-- show that index on parent is invalid before attaching partition indices for all partitions
+CREATE INDEX partitioning_test_2011_idx ON ONLY partitioning_test_2011 USING btree (id);
+CREATE INDEX partitioning_test_2012_idx ON ONLY partitioning_test_2012 USING btree (id);
+CREATE INDEX partitioning_test_2013_idx ON ONLY partitioning_test_2013 USING btree (id);
+SELECT indisvalid FROM pg_index WHERE indexrelid::regclass = 'partition_only_parent_index'::regclass;
+ indisvalid
+---------------------------------------------------------------------
+ f
+(1 row)
+
+-- show that index on parent becomes valid after attaching all partition indices
+ALTER INDEX partition_only_parent_index ATTACH PARTITION partitioning_test_2011_idx;
+ALTER INDEX partition_only_parent_index ATTACH PARTITION partitioning_test_2012_idx;
+ALTER INDEX partition_only_parent_index ATTACH PARTITION partitioning_test_2013_idx;
+SELECT indisvalid FROM pg_index WHERE indexrelid::regclass = 'partition_only_parent_index'::regclass;
+ indisvalid
+---------------------------------------------------------------------
+ t
+(1 row)
+
+-- show that creating new partitions gets the index by default and the index is still valid
+CREATE TABLE partitioning_test_2014 PARTITION OF partitioning_test FOR VALUES FROM ('2014-01-01') TO ('2015-01-01');
+SELECT count(*) FROM pg_index WHERE indexrelid::regclass::text LIKE 'partitioning_test_2014%';
+ count
+---------------------------------------------------------------------
+     1
+(1 row)
+
+SELECT indisvalid FROM pg_index WHERE indexrelid::regclass = 'partition_only_parent_index'::regclass;
+ indisvalid
+---------------------------------------------------------------------
+ t
+(1 row)
+
+\c - - - :worker_1_port
+-- Show that partitioned index is not visible on the MX worker node
+select citus_table_is_visible(indexrelid::regclass) from pg_index where indexrelid::regclass::text LIKE 'partition_only_parent_index_%' limit 1;
+ citus_table_is_visible
+---------------------------------------------------------------------
+ f
+(1 row)
+
+\c - - - :master_port
+DROP INDEX partition_only_parent_index;
 -- make sure we can repeatedly call start_metadata_sync_to_node
 SELECT start_metadata_sync_to_node('localhost', :worker_1_port);
  start_metadata_sync_to_node

src/test/regress/output/multi_alter_table_statements.source

@@ -1190,7 +1190,7 @@ 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;
 ERROR:  alter index ... set tablespace ... is currently unsupported
-DETAIL:  Only RENAME TO, SET (), RESET () and SET STATISTICS are supported.
+DETAIL:  Only RENAME TO, SET (), RESET (), ATTACH PARTITION and SET STATISTICS 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

src/test/regress/sql/multi_index_statements.sql

@@ -279,6 +279,72 @@ CREATE INDEX f1
     ON test_index_creation1 USING btree
     (field1);
 
+-- should be able to create index only for parent on both
+-- coordinator and worker nodes
+CREATE INDEX parent_index
+    ON ONLY test_index_creation1 USING btree
+    (field1);
+
+-- show that we have parent index only on the parent table not on the partitions
+SELECT count(*) FROM pg_index WHERE indrelid::regclass::text = 'test_index_creation1' AND indexrelid::regclass::text = 'parent_index';
+SELECT count(*) FROM pg_index WHERE indrelid::regclass::text LIKE 'test_index_creation1_p2020%' AND indexrelid::regclass::text LIKE 'parent_index%';
+
+\c - - - :worker_1_port
+SET search_path TO multi_index_statements;
+
+-- show that we have parent index_* only on the parent shards not on the partition shards
+SELECT count(*) FROM pg_index WHERE indrelid::regclass::text LIKE 'test_index_creation1_%' AND indexrelid::regclass::text LIKE 'parent_index%';
+SELECT count(*) FROM pg_index WHERE indrelid::regclass::text LIKE 'test_index_creation1_p2020%' AND indexrelid::regclass::text LIKE 'parent_index%';
+
+\c - - - :master_port
+SET search_path TO multi_index_statements;
+
+-- attach child index of a partition to parent index of the partitioned table
+CREATE INDEX child_index
+    ON test_index_creation1_p2020_09_26 USING btree
+    (field1);
+
+ALTER INDEX parent_index ATTACH PARTITION child_index;
+
+-- show that child index inherits from parent index which means it is attached to it
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass::text = 'child_index' AND inhparent::regclass::text = 'parent_index';
+
+\c - - - :worker_1_port
+SET search_path TO multi_index_statements;
+
+-- show that child indices of partition shards also inherit from parent indices of parent shards
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass::text LIKE 'child_index%' AND inhparent::regclass::text LIKE 'parent_index%';
+
+\c - - - :master_port
+SET search_path TO multi_index_statements;
+
+-- verify error check for partitioned index
+ALTER INDEX parent_index SET TABLESPACE foo;
+
+-- drop parent index and show that child index will also be dropped
+DROP INDEX parent_index;
+SELECT count(*) FROM pg_index where indexrelid::regclass::text = 'child_index';
+
+-- show that having a foreign key to reference table causes sequential execution mode
+-- with ALTER INDEX ... ATTACH PARTITION
+
+CREATE TABLE index_creation_reference_table (id int primary key);
+SELECT create_reference_table('index_creation_reference_table');
+ALTER TABLE test_index_creation1 ADD CONSTRAINT foreign_key_to_ref_table
+    FOREIGN KEY (tenant_id)
+    REFERENCES index_creation_reference_table (id);
+
+CREATE INDEX parent_index ON ONLY test_index_creation1 USING btree (field1);
+CREATE INDEX child_index ON test_index_creation1_p2020_09_26 USING btree (field1);
+
+BEGIN;
+    show citus.multi_shard_modify_mode;
+    ALTER INDEX parent_index ATTACH PARTITION child_index;
+    show citus.multi_shard_modify_mode;
+ROLLBACK;
+
+DROP TABLE index_creation_reference_table CASCADE;
+
 SELECT
 'CREATE TABLE distributed_table(' ||
 string_Agg('col' || x::text || ' int,', ' ') ||

src/test/regress/sql/multi_mx_partitioning.sql

@@ -157,6 +157,51 @@ DROP TABLE partitioning_test;
 
 \c - - - :master_port
 
+-- Make sure that creating index on only parent and child won't form any inheritance but it will
+-- be formed after attaching child index to parent index
+CREATE INDEX partitioning_test_2010_idx ON ONLY partitioning_test_2010 USING btree (id);
+
+-- Show that there is no inheritance relation for the index above
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass = 'partitioning_test_2010_idx'::regclass;
+
+-- Now add the index only on parent and show that there won't be any inheritance for those indices
+CREATE INDEX partition_only_parent_index ON ONLY partitioning_test USING btree (id);
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass = 'partitioning_test_2010_idx'::regclass;
+
+-- Now attach the child index to parent and show there is inheritance
+ALTER INDEX partition_only_parent_index ATTACH PARTITION partitioning_test_2010_idx;
+SELECT count(*) FROM pg_inherits WHERE inhrelid::regclass = 'partitioning_test_2010_idx'::regclass;
+
+-- show that index on parent is invalid before attaching partition indices for all partitions
+CREATE INDEX partitioning_test_2011_idx ON ONLY partitioning_test_2011 USING btree (id);
+CREATE INDEX partitioning_test_2012_idx ON ONLY partitioning_test_2012 USING btree (id);
+CREATE INDEX partitioning_test_2013_idx ON ONLY partitioning_test_2013 USING btree (id);
+
+SELECT indisvalid FROM pg_index WHERE indexrelid::regclass = 'partition_only_parent_index'::regclass;
+
+-- show that index on parent becomes valid after attaching all partition indices
+ALTER INDEX partition_only_parent_index ATTACH PARTITION partitioning_test_2011_idx;
+ALTER INDEX partition_only_parent_index ATTACH PARTITION partitioning_test_2012_idx;
+ALTER INDEX partition_only_parent_index ATTACH PARTITION partitioning_test_2013_idx;
+
+SELECT indisvalid FROM pg_index WHERE indexrelid::regclass = 'partition_only_parent_index'::regclass;
+
+-- show that creating new partitions gets the index by default and the index is still valid
+CREATE TABLE partitioning_test_2014 PARTITION OF partitioning_test FOR VALUES FROM ('2014-01-01') TO ('2015-01-01');
+
+SELECT count(*) FROM pg_index WHERE indexrelid::regclass::text LIKE 'partitioning_test_2014%';
+
+SELECT indisvalid FROM pg_index WHERE indexrelid::regclass = 'partition_only_parent_index'::regclass;
+
+\c - - - :worker_1_port
+
+-- Show that partitioned index is not visible on the MX worker node
+select citus_table_is_visible(indexrelid::regclass) from pg_index where indexrelid::regclass::text LIKE 'partition_only_parent_index_%' limit 1;
+
+\c - - - :master_port
+
+DROP INDEX partition_only_parent_index;
+
 -- make sure we can repeatedly call start_metadata_sync_to_node
 SELECT start_metadata_sync_to_node('localhost', :worker_1_port);
 SELECT start_metadata_sync_to_node('localhost', :worker_1_port);