From 385ba94d15bc01b51529acf29c25814821a67918 Mon Sep 17 00:00:00 2001 From: naisila Date: Thu, 21 Oct 2021 14:15:15 +0300 Subject: [PATCH] Run fix_partition_shard_index_names after each wrong naming command --- src/backend/distributed/commands/table.c | 79 ++++ .../distributed/commands/utility_hook.c | 32 ++ .../utils/multi_partitioning_utils.c | 266 ++++++++----- src/include/distributed/commands.h | 2 + .../distributed/multi_partitioning_utils.h | 2 +- src/test/regress/create_schedule | 1 + .../expected/citus_local_table_triggers.out | 1 + .../expected/coordinator_shouldhaveshards.out | 6 +- ...lation_fix_partition_shard_index_names.out | 174 ++++++++ .../multi_fix_partition_shard_index_names.out | 372 +++++++++--------- .../regress/expected/multi_partitioning.out | 30 +- .../expected/partitioned_indexes_create.out | 80 ++++ .../expected/partitioning_issue_3970.out | 4 +- ...ation_fix_partition_shard_index_names.spec | 36 ++ .../multi_fix_partition_shard_index_names.sql | 189 +++++---- src/test/regress/sql/multi_partitioning.sql | 14 +- .../sql/partitioned_indexes_create.sql | 76 ++++ 17 files changed, 966 insertions(+), 398 deletions(-) create mode 100644 src/test/regress/expected/partitioned_indexes_create.out create mode 100644 src/test/regress/sql/partitioned_indexes_create.sql diff --git a/src/backend/distributed/commands/table.c b/src/backend/distributed/commands/table.c index 7a08ccda3..0f4a3dd7a 100644 --- a/src/backend/distributed/commands/table.c +++ b/src/backend/distributed/commands/table.c @@ -1625,6 +1625,18 @@ AlterTableCommandTypeIsTrigger(AlterTableType alterTableType) } +/* + * ConstrTypeUsesIndex returns true if the given constraint type uses an index + */ +bool +ConstrTypeUsesIndex(ConstrType constrType) +{ + return constrType == CONSTR_PRIMARY || + constrType == CONSTR_UNIQUE || + constrType == CONSTR_EXCLUSION; +} + + /* * AlterTableDropsForeignKey returns true if the given AlterTableStmt drops * a foreign key. False otherwise. @@ -2125,6 +2137,73 @@ PostprocessAlterTableStmt(AlterTableStmt *alterTableStatement) } +/* + * FixAlterTableStmtIndexNames runs after the ALTER TABLE command + * has already run on the coordinator, and also after the distributed DDL + * Jobs have been executed on the workers. + * + * We might have wrong index names generated on indexes of shards of partitions, + * see https://github.com/citusdata/citus/pull/5397 for the details. So we + * perform the relevant checks and index renaming here. + */ +void +FixAlterTableStmtIndexNames(AlterTableStmt *alterTableStatement) +{ + LOCKMODE lockmode = AlterTableGetLockLevel(alterTableStatement->cmds); + Oid relationId = AlterTableLookupRelation(alterTableStatement, lockmode); + if (!(OidIsValid(relationId) && IsCitusTable(relationId) && + PartitionedTable(relationId))) + { + /* we are only interested in partitioned Citus tables */ + return; + } + + List *commandList = alterTableStatement->cmds; + AlterTableCmd *command = NULL; + foreach_ptr(command, commandList) + { + AlterTableType alterTableType = command->subtype; + + /* + * If this a partitioned table, and the constraint type uses an index + * UNIQUE, PRIMARY KEY, EXCLUDE constraint, + * we have wrong index names generated on indexes of shards of + * partitions of this table, so we should fix them + */ + Constraint *constraint = (Constraint *) command->def; + if (alterTableType == AT_AddConstraint && + ConstrTypeUsesIndex(constraint->contype)) + { + bool missingOk = false; + const char *constraintName = constraint->conname; + Oid constraintId = + get_relation_constraint_oid(relationId, constraintName, missingOk); + + /* fix only the relevant index */ + Oid parentIndexOid = get_constraint_index(constraintId); + + FixPartitionShardIndexNames(relationId, parentIndexOid); + } + /* + * If this is an ALTER TABLE .. ATTACH PARTITION command + * we have wrong index names generated on indexes of shards of + * the current partition being attached, so we should fix them + */ + else if (alterTableType == AT_AttachPartition) + { + PartitionCmd *partitionCommand = (PartitionCmd *) command->def; + bool partitionMissingOk = false; + Oid partitionRelationId = + RangeVarGetRelid(partitionCommand->name, lockmode, + partitionMissingOk); + Oid parentIndexOid = InvalidOid; /* fix all the indexes */ + + FixPartitionShardIndexNames(partitionRelationId, parentIndexOid); + } + } +} + + /* * GetSequenceOid returns the oid of the sequence used as default value * of the attribute with given attnum of the given table relationId diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index 809ea8650..66f749ecf 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -649,6 +649,17 @@ ProcessUtilityInternal(PlannedStmt *pstmt, ExecuteDistributedDDLJob(ddlJob); } + if (IsA(parsetree, AlterTableStmt)) + { + /* + * This postprocess needs to be done after the distributed ddl jobs have + * been executed in the workers, hence is separate from PostprocessAlterTableStmt. + * We might have wrong index names generated on indexes of shards of partitions, + * so we perform the relevant checks and index renaming here. + */ + FixAlterTableStmtIndexNames(castNode(AlterTableStmt, parsetree)); + } + /* * For CREATE/DROP/REINDEX CONCURRENTLY we mark the index as valid * after successfully completing the distributed DDL job. @@ -662,6 +673,27 @@ ProcessUtilityInternal(PlannedStmt *pstmt, /* no failures during CONCURRENTLY, mark the index as valid */ MarkIndexValid(indexStmt); } + + /* + * We make sure schema name is not null in the PreprocessIndexStmt. + */ + Oid schemaId = get_namespace_oid(indexStmt->relation->schemaname, true); + Oid relationId = get_relname_relid(indexStmt->relation->relname, schemaId); + + /* + * If this a partitioned table, and CREATE INDEX was not run with ONLY, + * we have wrong index names generated on indexes of shards of + * partitions of this table, so we should fix them. + */ + if (IsCitusTable(relationId) && PartitionedTable(relationId) && + indexStmt->relation->inh) + { + /* only fix this specific index */ + Oid indexRelationId = + get_relname_relid(indexStmt->idxname, schemaId); + + FixPartitionShardIndexNames(relationId, indexRelationId); + } } } diff --git a/src/backend/distributed/utils/multi_partitioning_utils.c b/src/backend/distributed/utils/multi_partitioning_utils.c index ee6e13571..c0751a49b 100644 --- a/src/backend/distributed/utils/multi_partitioning_utils.c +++ b/src/backend/distributed/utils/multi_partitioning_utils.c @@ -53,15 +53,20 @@ static Relation try_relation_open_nolock(Oid relationId); static List * CreateFixPartitionConstraintsTaskList(Oid relationId); static List * WorkerFixPartitionConstraintCommandList(Oid relationId, uint64 shardId, List *checkConstraintList); -static List * CreateFixPartitionShardIndexNamesTaskList(Oid parentRelationId); +static List * CreateFixPartitionShardIndexNamesTaskList(Oid parentRelationId, + Oid partitionRelationId, + Oid parentIndexOid); static List * WorkerFixPartitionShardIndexNamesCommandList(uint64 parentShardId, - List *indexIdList); + List *indexIdList, + Oid partitionRelationId); static List * WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex( - char *qualifiedParentShardIndexName, Oid parentIndexId); + char *qualifiedParentShardIndexName, Oid parentIndexId, Oid partitionRelationId); static List * WorkerFixPartitionShardIndexNamesCommandListForPartitionIndex(Oid partitionIndexId, char * - qualifiedParentShardIndexName); + qualifiedParentShardIndexName, + Oid + partitionId); static List * CheckConstraintNameListForRelation(Oid relationId); static bool RelationHasConstraint(Oid relationId, char *constraintName); static char * RenameConstraintCommand(Oid relationId, char *constraintName, @@ -149,7 +154,8 @@ worker_fix_pre_citus10_partitioned_table_constraint_names(PG_FUNCTION_ARGS) /* * fix_partition_shard_index_names fixes the index names of shards of partitions of - * partitioned tables on workers. + * partitioned tables on workers. If the input is a partition rather than a partitioned + * table, we only fix the index names of shards of that particular partition. * * When running CREATE INDEX on parent_table, we didn't explicitly create the index on * each partition as well. Postgres created indexes for partitions in the coordinator, @@ -163,6 +169,7 @@ worker_fix_pre_citus10_partitioned_table_constraint_names(PG_FUNCTION_ARGS) * fix_partition_shard_index_names renames indexes of shards of partition tables to include * the shardId at the end of the name, regardless of whether index name was long or short * As a result there will be no index name ending in _idx, rather all will end in _{shardid} + * * Algorithm is: * foreach parentShard in shardListOfParentTableId: * foreach parentIndex on parent: @@ -186,48 +193,17 @@ fix_partition_shard_index_names(PG_FUNCTION_ARGS) EnsureCoordinator(); Oid relationId = PG_GETARG_OID(0); - - Relation relation = try_relation_open(relationId, AccessExclusiveLock); - - if (relation == NULL) - { - ereport(NOTICE, (errmsg("relation with OID %u does not exist, skipping", - relationId))); - PG_RETURN_VOID(); - } - - if (relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) - { - relation_close(relation, NoLock); - ereport(ERROR, (errmsg( - "Fixing shard index names is only applicable to partitioned" - " tables, and \"%s\" is not a partitioned table", - RelationGetRelationName(relation)))); - } + Oid parentIndexOid = InvalidOid; /* fix all the indexes */ if (!IsCitusTable(relationId)) { - relation_close(relation, NoLock); - ereport(ERROR, (errmsg("fix_partition_shard_index_names can " - "only be called for distributed partitioned tables"))); + ereport(ERROR, (errmsg("fix_partition_shard_index_names can only be called " + "for Citus tables"))); } EnsureTableOwner(relationId); - List *taskList = CreateFixPartitionShardIndexNamesTaskList(relationId); - - /* do not do anything if there are no index names to fix */ - if (taskList != NIL) - { - bool localExecutionSupported = true; - RowModifyLevel modLevel = ROW_MODIFY_NONE; - ExecutionParams *execParams = CreateBasicExecutionParams(modLevel, taskList, - MaxAdaptiveExecutorPoolSize, - localExecutionSupported); - ExecuteTaskListExtended(execParams); - } - - relation_close(relation, NoLock); + FixPartitionShardIndexNames(relationId, parentIndexOid); PG_RETURN_VOID(); } @@ -308,6 +284,67 @@ worker_fix_partition_shard_index_names(PG_FUNCTION_ARGS) } +/* + * FixPartitionShardIndexNames gets a relationId. The input relationId should be + * either a parent or partition table. If it is a parent table, then all the + * index names on all the partitions are fixed. If it is a partition, only the + * specific partition is fixed. + * + * The second parentIndexOid parameter is optional. If provided a valid Oid, only + * that specific index name is fixed. + */ +void +FixPartitionShardIndexNames(Oid relationId, Oid parentIndexOid) +{ + Relation relation = try_relation_open(relationId, AccessShareLock); + + if (relation == NULL) + { + ereport(NOTICE, (errmsg("relation with OID %u does not exist, skipping", + relationId))); + return; + } + + /* at this point, we should only be dealing with Citus tables */ + Assert(IsCitusTable(relationId)); + + Oid parentRelationId = InvalidOid; + Oid partitionRelationId = InvalidOid; + + if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + parentRelationId = relationId; + } + else if (PartitionTable(relationId)) + { + parentRelationId = PartitionParentOid(relationId); + partitionRelationId = relationId; + } + else + { + relation_close(relation, NoLock); + ereport(ERROR, (errmsg("Fixing shard index names is only applicable to " + "partitioned tables or partitions, " + "and \"%s\" is neither", + RelationGetRelationName(relation)))); + } + + List *taskList = + CreateFixPartitionShardIndexNamesTaskList(parentRelationId, + partitionRelationId, + parentIndexOid); + + /* do not do anything if there are no index names to fix */ + if (taskList != NIL) + { + bool localExecutionSupported = true; + ExecuteUtilityTaskList(taskList, localExecutionSupported); + } + + relation_close(relation, NoLock); +} + + /* * CreateFixPartitionConstraintsTaskList goes over all the partitions of a distributed * partitioned table, and creates the list of tasks to execute @@ -436,72 +473,110 @@ WorkerFixPartitionConstraintCommandList(Oid relationId, uint64 shardId, /* - * CreateFixPartitionShardIndexNamesTaskList goes over all the indexes of a distributed - * partitioned table, and creates the list of tasks to execute - * worker_fix_partition_shard_index_names UDF on worker nodes. + * CreateFixPartitionShardIndexNamesTaskList goes over all the + * indexes of a distributed partitioned table unless parentIndexOid + * is valid. If it is valid, only the given index is processed. * - * We create parent_table_shard_count tasks, - * each task with parent_indexes_count x parent_partitions_count query strings. + * The function creates the list of tasks to execute + * worker_fix_partition_shard_index_names() on worker nodes. + * + * When the partitionRelationId is a valid Oid, the function only operates on the + * given partition. Otherwise, the function create tasks for all the partitions. + * + * So, for example, if a new partition is created, we only need to fix only for the + * new partition, hence partitionRelationId should be a valid Oid. However, if a new + * index/constraint is created on the parent, we should fix all the partitions, hence + * partitionRelationId should be InvalidOid. + * + * As a reflection of the above, we always create parent_table_shard_count tasks. + * When we need to fix all the partitions, each task with parent_indexes_count + * times partition_count query strings. When we need to fix a single + * partition each task will have parent_indexes_count query strings. When we need + * to fix a single index, parent_indexes_count becomes 1. */ static List * -CreateFixPartitionShardIndexNamesTaskList(Oid parentRelationId) +CreateFixPartitionShardIndexNamesTaskList(Oid parentRelationId, Oid partitionRelationId, + Oid parentIndexOid) { - List *taskList = NIL; - - /* enumerate the tasks when putting them to the taskList */ - int taskId = 1; + List *partitionList = PartitionList(parentRelationId); + if (partitionList == NIL) + { + /* early exit if the parent relation does not have any partitions */ + return NIL; + } Relation parentRelation = RelationIdGetRelation(parentRelationId); + List *parentIndexIdList = NIL; - List *parentIndexIdList = RelationGetIndexList(parentRelation); + if (parentIndexOid != InvalidOid) + { + parentIndexIdList = list_make1_oid(parentIndexOid); + } + else + { + parentIndexIdList = RelationGetIndexList(parentRelation); + } - /* early exit if the parent relation does not have any indexes */ if (parentIndexIdList == NIL) { + /* early exit if the parent relation does not have any indexes */ RelationClose(parentRelation); return NIL; } - List *partitionList = PartitionList(parentRelationId); - - /* early exit if the parent relation does not have any partitions */ - if (partitionList == NIL) + /* + * Lock shard metadata, if a specific partition is provided, lock that. Otherwise, + * lock all partitions. + */ + if (OidIsValid(partitionRelationId)) { - RelationClose(parentRelation); - return NIL; + /* if a partition was provided we only need to lock that partition's metadata */ + List *partitionShardIntervalList = LoadShardIntervalList(partitionRelationId); + LockShardListMetadata(partitionShardIntervalList, ShareLock); + } + else + { + Oid partitionId = InvalidOid; + foreach_oid(partitionId, partitionList) + { + List *partitionShardIntervalList = LoadShardIntervalList(partitionId); + LockShardListMetadata(partitionShardIntervalList, ShareLock); + } } List *parentShardIntervalList = LoadShardIntervalList(parentRelationId); /* lock metadata before getting placement lists */ LockShardListMetadata(parentShardIntervalList, ShareLock); - Oid partitionId = InvalidOid; - foreach_oid(partitionId, partitionList) - { - List *partitionShardIntervalList = LoadShardIntervalList(partitionId); - LockShardListMetadata(partitionShardIntervalList, ShareLock); - } + + int taskId = 1; + List *taskList = NIL; ShardInterval *parentShardInterval = NULL; foreach_ptr(parentShardInterval, parentShardIntervalList) { uint64 parentShardId = parentShardInterval->shardId; - List *queryStringList = WorkerFixPartitionShardIndexNamesCommandList( - parentShardId, parentIndexIdList); + List *queryStringList = + WorkerFixPartitionShardIndexNamesCommandList(parentShardId, + parentIndexIdList, + partitionRelationId); - Task *task = CitusMakeNode(Task); - task->jobId = INVALID_JOB_ID; - task->taskId = taskId++; + if (queryStringList != NIL) + { + Task *task = CitusMakeNode(Task); + task->jobId = INVALID_JOB_ID; + task->taskId = taskId++; - task->taskType = DDL_TASK; - SetTaskQueryStringList(task, queryStringList); - task->dependentTaskList = NULL; - task->replicationModel = REPLICATION_MODEL_INVALID; - task->anchorShardId = parentShardId; - task->taskPlacementList = ActiveShardPlacementList(parentShardId); + task->taskType = DDL_TASK; + SetTaskQueryStringList(task, queryStringList); + task->dependentTaskList = NULL; + task->replicationModel = REPLICATION_MODEL_INVALID; + task->anchorShardId = parentShardId; + task->taskPlacementList = ActiveShardPlacementList(parentShardId); - taskList = lappend(taskList, task); + taskList = lappend(taskList, task); + } } RelationClose(parentRelation); @@ -516,7 +591,8 @@ CreateFixPartitionShardIndexNamesTaskList(Oid parentRelationId) */ static List * WorkerFixPartitionShardIndexNamesCommandList(uint64 parentShardId, - List *parentIndexIdList) + List *parentIndexIdList, + Oid partitionRelationId) { List *commandList = NIL; Oid parentIndexId = InvalidOid; @@ -539,7 +615,7 @@ WorkerFixPartitionShardIndexNamesCommandList(uint64 parentShardId, char *qualifiedParentShardIndexName = quote_qualified_identifier(schemaName, parentShardIndexName); List *commands = WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex( - qualifiedParentShardIndexName, parentIndexId); + qualifiedParentShardIndexName, parentIndexId, partitionRelationId); commandList = list_concat(commandList, commands); } @@ -548,12 +624,17 @@ WorkerFixPartitionShardIndexNamesCommandList(uint64 parentShardId, /* - * WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex creates a list of queries that will fix - * all child index names of given index on shard of parent partitioned table. + * WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex creates a list + * of queries that will fix the child index names of given index on shard + * of parent partitioned table. + * + * In case a partition was provided as argument (partitionRelationId isn't InvalidOid) + * the list of queries will include only the child indexes whose relation is the + * given partition. Otherwise, all the partitions are included. */ static List * WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex( - char *qualifiedParentShardIndexName, Oid parentIndexId) + char *qualifiedParentShardIndexName, Oid parentIndexId, Oid partitionRelationId) { List *commandList = NIL; @@ -561,14 +642,21 @@ WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex( * Get the list of all partition indexes that are children of current * index on parent */ - List *partitionIndexIds = find_inheritance_children(parentIndexId, - ShareRowExclusiveLock); + List *partitionIndexIds = + find_inheritance_children(parentIndexId, ShareRowExclusiveLock); + + bool addAllPartitions = (partitionRelationId == InvalidOid); Oid partitionIndexId = InvalidOid; foreach_oid(partitionIndexId, partitionIndexIds) { - List *commands = WorkerFixPartitionShardIndexNamesCommandListForPartitionIndex( - partitionIndexId, qualifiedParentShardIndexName); - commandList = list_concat(commandList, commands); + Oid partitionId = IndexGetRelation(partitionIndexId, false); + if (addAllPartitions || partitionId == partitionRelationId) + { + List *commands = + WorkerFixPartitionShardIndexNamesCommandListForPartitionIndex( + partitionIndexId, qualifiedParentShardIndexName, partitionId); + commandList = list_concat(commandList, commands); + } } return commandList; } @@ -577,18 +665,18 @@ WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex( /* * WorkerFixPartitionShardIndexNamesCommandListForPartitionIndex creates a list of queries that will fix * all child index names of given index on shard of parent partitioned table, whose table relation is a shard - * of the partition that is the table relation of given partitionIndexId + * of the partition that is the table relation of given partitionIndexId, which is partitionId */ static List * WorkerFixPartitionShardIndexNamesCommandListForPartitionIndex(Oid partitionIndexId, char * - qualifiedParentShardIndexName) + qualifiedParentShardIndexName, + Oid partitionId) { List *commandList = NIL; /* get info for this partition relation of this index*/ char *partitionIndexName = get_rel_name(partitionIndexId); - Oid partitionId = IndexGetRelation(partitionIndexId, false); char *partitionName = get_rel_name(partitionId); char *partitionSchemaName = get_namespace_name(get_rel_namespace(partitionId)); List *partitionShardIntervalList = LoadShardIntervalList(partitionId); diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 9c0c86c5b..d57db6f9c 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -414,6 +414,7 @@ extern Node * SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTa extern bool IsAlterTableRenameStmt(RenameStmt *renameStmt); extern void ErrorIfAlterDropsPartitionColumn(AlterTableStmt *alterTableStatement); extern void PostprocessAlterTableStmt(AlterTableStmt *pStmt); +extern void FixAlterTableStmtIndexNames(AlterTableStmt *pStmt); extern void ErrorUnsupportedAlterTableAddColumn(Oid relationId, AlterTableCmd *command, Constraint *constraint); extern void ErrorIfUnsupportedConstraint(Relation relation, char distributionMethod, @@ -423,6 +424,7 @@ extern ObjectAddress AlterTableSchemaStmtObjectAddress(Node *stmt, bool missing_ok); extern List * MakeNameListFromRangeVar(const RangeVar *rel); extern Oid GetSequenceOid(Oid relationId, AttrNumber attnum); +extern bool ConstrTypeUsesIndex(ConstrType constrType); /* truncate.c - forward declarations */ diff --git a/src/include/distributed/multi_partitioning_utils.h b/src/include/distributed/multi_partitioning_utils.h index f04cbd584..9f905b19d 100644 --- a/src/include/distributed/multi_partitioning_utils.h +++ b/src/include/distributed/multi_partitioning_utils.h @@ -29,6 +29,6 @@ extern List * GenerateAttachPartitionCommandRelationIdList(List *relationIds); extern char * GeneratePartitioningInformation(Oid tableId); extern void FixPartitionConstraintsOnWorkers(Oid relationId); extern void FixLocalPartitionConstraints(Oid relationId, int64 shardId); - +extern void FixPartitionShardIndexNames(Oid relationId, Oid parentIndexOid); #endif /* MULTI_PARTITIONING_UTILS_H_ */ diff --git a/src/test/regress/create_schedule b/src/test/regress/create_schedule index ecdec8a9b..50a299d7e 100644 --- a/src/test/regress/create_schedule +++ b/src/test/regress/create_schedule @@ -2,3 +2,4 @@ test: intermediate_result_pruning_create test: prepared_statements_create_load ch_benchmarks_create_load test: dropped_columns_create_load distributed_planning_create_load test: local_dist_join_load +test: partitioned_indexes_create diff --git a/src/test/regress/expected/citus_local_table_triggers.out b/src/test/regress/expected/citus_local_table_triggers.out index 546a16dc9..8d9491933 100644 --- a/src/test/regress/expected/citus_local_table_triggers.out +++ b/src/test/regress/expected/citus_local_table_triggers.out @@ -457,6 +457,7 @@ ALTER TABLE par_another_citus_local_table ADD CONSTRAINT fkey_self FOREIGN KEY(v ALTER TABLE par_citus_local_table ADD CONSTRAINT fkey_c_to_c FOREIGN KEY(val) REFERENCES par_another_citus_local_table(val) ON UPDATE CASCADE; SELECT citus_add_local_table_to_metadata('par_another_citus_local_table', cascade_via_foreign_keys=>true); NOTICE: executing the command locally: SELECT worker_apply_inter_shard_ddl_command (1507011, 'citus_local_table_triggers', 1507012, 'citus_local_table_triggers', 'ALTER TABLE citus_local_table_triggers.par_another_citus_local_table ATTACH PARTITION citus_local_table_triggers.par_another_citus_local_table_1 FOR VALUES FROM (1) TO (10000);') +NOTICE: executing the command locally: SELECT worker_fix_partition_shard_index_names('citus_local_table_triggers.par_another_citus_local_table_val_key_1507011'::regclass, 'citus_local_table_triggers.par_another_citus_local_table_1_1507012', 'par_another_citus_local_table_1_val_key_1507012') NOTICE: executing the command locally: SELECT worker_apply_inter_shard_ddl_command (1507013, 'citus_local_table_triggers', 1507014, 'citus_local_table_triggers', 'ALTER TABLE citus_local_table_triggers.par_citus_local_table ATTACH PARTITION citus_local_table_triggers.par_citus_local_table_1 FOR VALUES FROM (1) TO (10000);') NOTICE: executing the command locally: SELECT worker_apply_inter_shard_ddl_command (1507011, 'citus_local_table_triggers', 1507011, 'citus_local_table_triggers', 'ALTER TABLE citus_local_table_triggers.par_another_citus_local_table ADD CONSTRAINT fkey_self FOREIGN KEY (val) REFERENCES citus_local_table_triggers.par_another_citus_local_table(val)') NOTICE: executing the command locally: SELECT worker_apply_inter_shard_ddl_command (1507013, 'citus_local_table_triggers', 1507011, 'citus_local_table_triggers', 'ALTER TABLE citus_local_table_triggers.par_citus_local_table ADD CONSTRAINT fkey_c_to_c FOREIGN KEY (val) REFERENCES citus_local_table_triggers.par_another_citus_local_table(val) ON UPDATE CASCADE') diff --git a/src/test/regress/expected/coordinator_shouldhaveshards.out b/src/test/regress/expected/coordinator_shouldhaveshards.out index dcae09dbe..1b5cf7051 100644 --- a/src/test/regress/expected/coordinator_shouldhaveshards.out +++ b/src/test/regress/expected/coordinator_shouldhaveshards.out @@ -624,8 +624,10 @@ select create_distributed_table('test_index_creation1', 'tenant_id'); CREATE INDEX ix_test_index_creation5 ON test_index_creation1 USING btree(tenant_id, timeperiod) INCLUDE (field1) WHERE (tenant_id = 100); -NOTICE: executing the command locally: CREATE INDEX ix_test_index_creation5_1503042 ON coordinator_shouldhaveshards.test_index_creation1_1503042 USING btree (tenant_id ,timeperiod ) INCLUDE (field1 )WHERE (tenant_id = 100) -NOTICE: executing the command locally: CREATE INDEX ix_test_index_creation5_1503045 ON coordinator_shouldhaveshards.test_index_creation1_1503045 USING btree (tenant_id ,timeperiod ) INCLUDE (field1 )WHERE (tenant_id = 100) +NOTICE: executing the command locally: CREATE INDEX ix_test_index_creation5_1503042 ON coordinator_shouldhaveshards.test_index_creation1_1503042 USING btree (tenant_id ,timeperiod ) INCLUDE (field1 ) WHERE (tenant_id = 100) +NOTICE: executing the command locally: CREATE INDEX ix_test_index_creation5_1503045 ON coordinator_shouldhaveshards.test_index_creation1_1503045 USING btree (tenant_id ,timeperiod ) INCLUDE (field1 ) WHERE (tenant_id = 100) +NOTICE: executing the command locally: SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503048', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503048');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503049', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503049');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503050', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503050');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503051', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503051');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503052', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503052');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503053', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503053');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503054', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503054');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503055', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503055');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503056', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503056');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503057', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503057');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503058', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503058');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503042'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503059', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503059') +NOTICE: executing the command locally: SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503048', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503048');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503049', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503049');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503050', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503050');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503051', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503051');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503052', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503052');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_26_1503053', 'test_index_creation1_p2020_09_2_tenant_id_time_6020e8f8_1503053');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503054', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503054');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503055', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503055');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503056', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503056');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503057', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503057');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503058', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503058');SELECT worker_fix_partition_shard_index_names('coordinator_shouldhaveshards.ix_test_index_creation5_1503045'::regclass, 'coordinator_shouldhaveshards.test_index_creation1_p2020_09_27_1503059', 'test_index_creation1_p2020_09__tenant_id_timep_624f7e94_1503059') -- test if indexes are created SELECT 1 AS created WHERE EXISTS(SELECT * FROM pg_indexes WHERE indexname LIKE '%test_index_creation%'); created diff --git a/src/test/regress/expected/isolation_fix_partition_shard_index_names.out b/src/test/regress/expected/isolation_fix_partition_shard_index_names.out index 9a7959386..5359eee69 100644 --- a/src/test/regress/expected/isolation_fix_partition_shard_index_names.out +++ b/src/test/regress/expected/isolation_fix_partition_shard_index_names.out @@ -52,3 +52,177 @@ step s2-commit: COMMIT; step s1-drop-table: <... completed> + +starting permutation: s2-begin s2-create-index s1-select-from-table s2-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s2-begin: + BEGIN; + +step s2-create-index: + CREATE INDEX ON dist_partitioned_table USING btree(a); + +step s1-select-from-table: + SELECT * FROM dist_partitioned_table; + +a|created_at +--------------------------------------------------------------------- +(0 rows) + +step s2-commit: + COMMIT; + + +starting permutation: s2-begin s2-create-index s1-insert-into-table s2-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s2-begin: + BEGIN; + +step s2-create-index: + CREATE INDEX ON dist_partitioned_table USING btree(a); + +step s1-insert-into-table: + INSERT INTO dist_partitioned_table VALUES (0, '2019-01-01'); + +step s2-commit: + COMMIT; + +step s1-insert-into-table: <... completed> + +starting permutation: s1-begin s1-select-from-table s2-create-index-concurrently s1-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s1-begin: + BEGIN; + +step s1-select-from-table: + SELECT * FROM dist_partitioned_table; + +a|created_at +--------------------------------------------------------------------- +(0 rows) + +step s2-create-index-concurrently: + CREATE INDEX CONCURRENTLY ON dist_partitioned_table USING btree(a); + +ERROR: cannot create index on partitioned table "dist_partitioned_table" concurrently +step s1-commit: + COMMIT; + + +starting permutation: s1-begin s1-insert-into-table s2-create-index-concurrently s1-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s1-begin: + BEGIN; + +step s1-insert-into-table: + INSERT INTO dist_partitioned_table VALUES (0, '2019-01-01'); + +step s2-create-index-concurrently: + CREATE INDEX CONCURRENTLY ON dist_partitioned_table USING btree(a); + +ERROR: cannot create index on partitioned table "dist_partitioned_table" concurrently +step s1-commit: + COMMIT; + + +starting permutation: s1-begin s1-select-from-table s2-create-index s1-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s1-begin: + BEGIN; + +step s1-select-from-table: + SELECT * FROM dist_partitioned_table; + +a|created_at +--------------------------------------------------------------------- +(0 rows) + +step s2-create-index: + CREATE INDEX ON dist_partitioned_table USING btree(a); + +step s1-commit: + COMMIT; + + +starting permutation: s1-begin s1-insert-into-table s2-create-index s1-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s1-begin: + BEGIN; + +step s1-insert-into-table: + INSERT INTO dist_partitioned_table VALUES (0, '2019-01-01'); + +step s2-create-index: + CREATE INDEX ON dist_partitioned_table USING btree(a); + +step s1-commit: + COMMIT; + +step s2-create-index: <... completed> + +starting permutation: s2-begin s2-create-index-concurrently s1-select-from-table s2-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s2-begin: + BEGIN; + +step s2-create-index-concurrently: + CREATE INDEX CONCURRENTLY ON dist_partitioned_table USING btree(a); + +ERROR: CREATE INDEX CONCURRENTLY cannot run inside a transaction block +step s1-select-from-table: + SELECT * FROM dist_partitioned_table; + +a|created_at +--------------------------------------------------------------------- +(0 rows) + +step s2-commit: + COMMIT; + + +starting permutation: s2-begin s2-create-index-concurrently s1-insert-into-table s2-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s2-begin: + BEGIN; + +step s2-create-index-concurrently: + CREATE INDEX CONCURRENTLY ON dist_partitioned_table USING btree(a); + +ERROR: CREATE INDEX CONCURRENTLY cannot run inside a transaction block +step s1-insert-into-table: + INSERT INTO dist_partitioned_table VALUES (0, '2019-01-01'); + +step s2-commit: + COMMIT; + diff --git a/src/test/regress/expected/multi_fix_partition_shard_index_names.out b/src/test/regress/expected/multi_fix_partition_shard_index_names.out index bab862c50..ba08130a6 100644 --- a/src/test/regress/expected/multi_fix_partition_shard_index_names.out +++ b/src/test/regress/expected/multi_fix_partition_shard_index_names.out @@ -33,12 +33,12 @@ SELECT create_distributed_table('not_partitioned', 'id'); (1 row) SELECT fix_partition_shard_index_names('not_partitioned'::regclass); -ERROR: Fixing shard index names is only applicable to partitioned tables, and "not_partitioned" is not a partitioned table +ERROR: Fixing shard index names is only applicable to partitioned tables or partitions, and "not_partitioned" is neither -- fix_partition_shard_index_names cannot be called for partitioned -- and not distributed tables CREATE TABLE not_distributed(created_at timestamptz) PARTITION BY RANGE (created_at); SELECT fix_partition_shard_index_names('not_distributed'::regclass); -ERROR: fix_partition_shard_index_names can only be called for distributed partitioned tables +ERROR: fix_partition_shard_index_names can only be called for Citus tables -- test with proper table CREATE TABLE dist_partitioned_table (dist_col int, another_col int, partition_col timestamp) PARTITION BY RANGE (partition_col); SELECT create_distributed_table('dist_partitioned_table', 'dist_col'); @@ -53,6 +53,8 @@ CREATE TABLE p PARTITION OF dist_partitioned_table FOR VALUES FROM ('2019-01-01' -- create an index on parent table -- we will see that it doesn't matter whether we name the index on parent or not -- indexes auto-generated on partitions will not use this name +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX short ON dist_partitioned_table USING btree (another_col, partition_col); SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; tablename | indexname @@ -63,34 +65,7 @@ SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' O (3 rows) \c - - - :worker_1_port --- Note that, the shell table from above partition_table_with_very_long_name --- and its shard partition_table_with_very_long_name_910008 --- have the same index name: partition_table_with_very_long_na_another_col_partition_col_idx -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - tablename | indexname ---------------------------------------------------------------------- - dist_partitioned_table_910004 | short_910004 - dist_partitioned_table_910006 | short_910006 - p_910012 | p_910012_another_col_partition_col_idx - p_910014 | p_910014_another_col_partition_col_idx - partition_table_with_very_long_name_910008 | partition_table_with_very_long_na_another_col_partition_col_idx - partition_table_with_very_long_name_910010 | partition_table_with_very_long_n_another_col_partition_col_idx1 -(6 rows) - -\c - - - :master_port --- this should fail because of the name clash explained above -SELECT start_metadata_sync_to_node('localhost', :worker_1_port); -ERROR: relation "partition_table_with_very_long_na_another_col_partition_col_idx" already exists -CONTEXT: while executing command on localhost:xxxxx --- let's fix the problematic table -SET search_path TO fix_idx_names, public; -SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); - fix_partition_shard_index_names ---------------------------------------------------------------------- - -(1 row) - -\c - - - :worker_1_port +-- the names are generated correctly -- shard id has been appended to all index names which didn't end in shard id -- this goes in line with Citus's way of naming indexes of shards: always append shardid to the end SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; @@ -105,15 +80,32 @@ SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' O (6 rows) \c - - - :master_port -SET search_path TO fix_idx_names, public; --- this should now work +-- this should work properly SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- (1 row) --- if we run this command again, the names will not change anymore since shardid is appended to them +\c - - - :worker_1_port +-- we have no clashes +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + dist_partitioned_table | short + dist_partitioned_table_910004 | short_910004 + dist_partitioned_table_910006 | short_910006 + p | p_another_col_partition_col_idx + p_910012 | p_another_col_partition_col_idx_910012 + p_910014 | p_another_col_partition_col_idx_910014 + partition_table_with_very_long_name | partition_table_with_very_long_na_another_col_partition_col_idx + partition_table_with_very_long_name_910008 | partition_table_with_very_long_na_another_col_p_dd884a3b_910008 + partition_table_with_very_long_name_910010 | partition_table_with_very_long_na_another_col_p_dd884a3b_910010 +(9 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +-- if we run this command again, the names will not change since shardid is appended to them SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); fix_partition_shard_index_names --------------------------------------------------------------------- @@ -242,10 +234,14 @@ DROP INDEX short; DROP TABLE yet_another_partition_table, another_partition_table_with_very_long_name; -- this will create constraint1 index on parent SET citus.max_adaptive_executor_pool_size TO 1; +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the ADD CONSTRAINT command ALTER TABLE dist_partitioned_table ADD CONSTRAINT constraint1 UNIQUE (dist_col, partition_col); RESET citus.max_adaptive_executor_pool_size; CREATE TABLE fk_table (id int, fk_column timestamp, FOREIGN KEY (id, fk_column) REFERENCES dist_partitioned_table (dist_col, partition_col)); -- try creating index to foreign key +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX ON dist_partitioned_table USING btree (dist_col, partition_col); SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; tablename | indexname @@ -259,34 +255,7 @@ SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' O (6 rows) \c - - - :worker_1_port --- index names don't end in shardid for partitions -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - tablename | indexname ---------------------------------------------------------------------- - dist_partitioned_table_910004 | constraint1_910004 - dist_partitioned_table_910004 | dist_partitioned_table_dist_col_partition_col_idx_910004 - dist_partitioned_table_910006 | constraint1_910006 - dist_partitioned_table_910006 | dist_partitioned_table_dist_col_partition_col_idx_910006 - p_910012 | p_910012_dist_col_partition_col_idx - p_910012 | p_910012_dist_col_partition_col_key - p_910014 | p_910014_dist_col_partition_col_idx - p_910014 | p_910014_dist_col_partition_col_key - partition_table_with_very_long_name_910008 | partition_table_with_very_long_name__dist_col_partition_col_idx - partition_table_with_very_long_name_910008 | partition_table_with_very_long_name__dist_col_partition_col_key - partition_table_with_very_long_name_910010 | partition_table_with_very_long_name_dist_col_partition_col_idx1 - partition_table_with_very_long_name_910010 | partition_table_with_very_long_name_dist_col_partition_col_key1 -(12 rows) - -\c - - - :master_port -SET search_path TO fix_idx_names, public; -SELECT fix_all_partition_shard_index_names(); - fix_all_partition_shard_index_names ---------------------------------------------------------------------- - dist_partitioned_table -(1 row) - -\c - - - :worker_1_port --- now index names end in shardid +-- index names end in shardid for partitions SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; tablename | indexname --------------------------------------------------------------------- @@ -317,6 +286,8 @@ CREATE INDEX short_parent ON ONLY dist_partitioned_table USING hash (dist_col); -- only another_partition will have the index on dist_col inherited from short_parent -- hence short_parent will still be invalid CREATE TABLE another_partition (dist_col int, another_col int, partition_col timestamp); +-- SELECT fix_partition_shard_index_names('another_partition') will be executed +-- automatically at the end of the ATTACH PARTITION command ALTER TABLE dist_partitioned_table ATTACH PARTITION another_partition FOR VALUES FROM ('2017-01-01') TO ('2018-01-01'); SELECT c.relname AS indexname FROM pg_catalog.pg_class c, pg_catalog.pg_namespace n, pg_catalog.pg_index i @@ -337,31 +308,7 @@ SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' O (3 rows) \c - - - :worker_1_port --- index names are already correct except for inherited index for another_partition -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - tablename | indexname ---------------------------------------------------------------------- - another_partition_361176 | another_partition_361176_dist_col_idx - another_partition_361178 | another_partition_361178_dist_col_idx - dist_partitioned_table_910004 | short_parent_910004 - dist_partitioned_table_910006 | short_parent_910006 - p_910012 | short_child_910012 - p_910014 | short_child_910014 -(6 rows) - -\c - - - :master_port -SET search_path TO fix_idx_names, public; --- this will fix inherited index for another_partition -SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); - fix_partition_shard_index_names ---------------------------------------------------------------------- - -(1 row) - --- this will error out becuase p is not partitioned, it is rather a partition -SELECT fix_partition_shard_index_names('p'::regclass); -ERROR: Fixing shard index names is only applicable to partitioned tables, and "p" is not a partitioned table -\c - - - :worker_1_port +-- index names are already correct, including inherited index for another_partition SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; tablename | indexname --------------------------------------------------------------------- @@ -378,9 +325,13 @@ SET search_path TO fix_idx_names, public; DROP INDEX short_parent; DROP INDEX short_child; DROP TABLE another_partition; --- expression indexes have the same problem with naming +-- try with expression indexes +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX expression_index ON dist_partitioned_table ((dist_col || ' ' || another_col)); -- try with statistics on index +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX statistics_on_index on dist_partitioned_table ((dist_col+another_col), (dist_col-another_col)); ALTER INDEX statistics_on_index ALTER COLUMN 1 SET STATISTICS 3737; ALTER INDEX statistics_on_index ALTER COLUMN 2 SET STATISTICS 3737; @@ -396,32 +347,7 @@ SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' O (6 rows) \c - - - :worker_1_port -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - tablename | indexname ---------------------------------------------------------------------- - dist_partitioned_table_910004 | expression_index_910004 - dist_partitioned_table_910004 | statistics_on_index_910004 - dist_partitioned_table_910006 | expression_index_910006 - dist_partitioned_table_910006 | statistics_on_index_910006 - p_910012 | p_910012_expr_expr1_idx - p_910012 | p_910012_expr_idx - p_910014 | p_910014_expr_expr1_idx - p_910014 | p_910014_expr_idx - partition_table_with_very_long_name_910008 | partition_table_with_very_long_name_910008_expr_expr1_idx - partition_table_with_very_long_name_910008 | partition_table_with_very_long_name_910008_expr_idx - partition_table_with_very_long_name_910010 | partition_table_with_very_long_name_910010_expr_expr1_idx - partition_table_with_very_long_name_910010 | partition_table_with_very_long_name_910010_expr_idx -(12 rows) - -\c - - - :master_port -SET search_path TO fix_idx_names, public; -SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); - fix_partition_shard_index_names ---------------------------------------------------------------------- - -(1 row) - -\c - - - :worker_1_port +-- we have correct names SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; tablename | indexname --------------------------------------------------------------------- @@ -487,6 +413,8 @@ SELECT create_distributed_table('dist_partitioned_table', 'dist_col'); -- create a partition with a long name CREATE TABLE partition_table_with_very_long_name PARTITION OF dist_partitioned_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); -- create an index on parent table +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX index_rep_factor_2 ON dist_partitioned_table USING btree (another_col, partition_col); SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; tablename | indexname @@ -496,29 +424,7 @@ SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' O (2 rows) \c - - - :worker_2_port -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - tablename | indexname ---------------------------------------------------------------------- - dist_partitioned_table_910050 | index_rep_factor_2_910050 - dist_partitioned_table_910051 | index_rep_factor_2_910051 - dist_partitioned_table_910052 | index_rep_factor_2_910052 - dist_partitioned_table_910053 | index_rep_factor_2_910053 - partition_table_with_very_long_name_910054 | partition_table_with_very_long_na_another_col_partition_col_idx - partition_table_with_very_long_name_910055 | partition_table_with_very_long_n_another_col_partition_col_idx1 - partition_table_with_very_long_name_910056 | partition_table_with_very_long_n_another_col_partition_col_idx2 - partition_table_with_very_long_name_910057 | partition_table_with_very_long_n_another_col_partition_col_idx3 -(8 rows) - -\c - - - :master_port --- let's fix the problematic table -SET search_path TO fix_idx_names, public; -SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); - fix_partition_shard_index_names ---------------------------------------------------------------------- - -(1 row) - -\c - - - :worker_2_port +-- index names are correct -- shard id has been appended to all index names which didn't end in shard id -- this goes in line with Citus's way of naming indexes of shards: always append shardid to the end SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; @@ -548,9 +454,8 @@ ERROR: permission denied for schema fix_idx_names RESET ROLE; SET search_path TO fix_idx_names, public; DROP TABLE dist_partitioned_table; --- also, we cannot do any further operations (e.g. rename) on the indexes of partitions because --- the index names on shards of partitions have been generated by Postgres, not Citus --- it doesn't matter here whether the partition name is long or short +-- We can do any further operations (e.g. rename) on the indexes of partitions because +-- the index names on shards of partitions have Citus naming, hence are reachable -- replicate scenario from above but this time with one shard so that this test isn't flaky SET citus.shard_count TO 1; SET citus.shard_replication_factor TO 1; @@ -564,55 +469,15 @@ SELECT create_distributed_table('dist_partitioned_table', 'dist_col'); CREATE TABLE partition_table_with_very_long_name PARTITION OF dist_partitioned_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); CREATE TABLE p PARTITION OF dist_partitioned_table FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX short ON dist_partitioned_table USING btree (another_col, partition_col); --- rename shouldn't work +-- rename works! ALTER INDEX partition_table_with_very_long_na_another_col_partition_col_idx RENAME TO partition_table_with_very_long_name_idx; -ERROR: relation "fix_idx_names.partition_table_with_very_long_na_another_col_p_dd884a3b_910031" does not exist -CONTEXT: while executing command on localhost:xxxxx --- we currently can't drop index on detached partition +-- we can drop index on detached partition -- https://github.com/citusdata/citus/issues/5138 ALTER TABLE dist_partitioned_table DETACH PARTITION p; DROP INDEX p_another_col_partition_col_idx; -ERROR: index "p_another_col_partition_col_idx_910032" does not exist -CONTEXT: while executing command on localhost:xxxxx --- let's reattach and retry after fixing index names -ALTER TABLE dist_partitioned_table ATTACH PARTITION p FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); -\c - - - :worker_1_port --- check the current broken index names -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - tablename | indexname ---------------------------------------------------------------------- - dist_partitioned_table_910030 | short_910030 - p_910032 | p_910032_another_col_partition_col_idx - partition_table_with_very_long_name_910031 | partition_table_with_very_long_na_another_col_partition_col_idx -(3 rows) - -\c - - - :master_port -SET search_path TO fix_idx_names, public; --- fix index names -SELECT fix_all_partition_shard_index_names(); - fix_all_partition_shard_index_names ---------------------------------------------------------------------- - dist_partitioned_table -(1 row) - -\c - - - :worker_1_port --- check the fixed index names -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - tablename | indexname ---------------------------------------------------------------------- - dist_partitioned_table_910030 | short_910030 - p_910032 | p_another_col_partition_col_idx_910032 - partition_table_with_very_long_name_910031 | partition_table_with_very_long_na_another_col_p_dd884a3b_910031 -(3 rows) - -\c - - - :master_port -SET search_path TO fix_idx_names, public; --- should now work -ALTER INDEX partition_table_with_very_long_na_another_col_partition_col_idx RENAME TO partition_table_with_very_long_name_idx; --- now we can drop index on detached partition -ALTER TABLE dist_partitioned_table DETACH PARTITION p; -DROP INDEX p_another_col_partition_col_idx; \c - - - :worker_1_port -- check that indexes have been renamed -- and that index on p has been dropped (it won't appear) @@ -625,13 +490,150 @@ SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' O \c - - - :master_port SET search_path TO fix_idx_names, public; +DROP TABLE dist_partitioned_table; +-- test with citus local table +SET client_min_messages TO WARNING; +SELECT 1 FROM citus_add_node('localhost', :master_port, groupid=>0); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +RESET client_min_messages; +CREATE TABLE date_partitioned_citus_local_table( + measureid integer, + eventdate date, + measure_data jsonb) PARTITION BY RANGE(eventdate); +SELECT citus_add_local_table_to_metadata('date_partitioned_citus_local_table'); + citus_add_local_table_to_metadata +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE partition_local_table PARTITION OF date_partitioned_citus_local_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); +-- SELECT fix_partition_shard_index_names('date_partitioned_citus_local_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE INDEX ON date_partitioned_citus_local_table USING btree(measureid); +-- check that index names are correct +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + date_partitioned_citus_local_table | date_partitioned_citus_local_table_measureid_idx + date_partitioned_citus_local_table_361377 | date_partitioned_citus_local_table_measureid_idx_361377 + partition_local_table | partition_local_table_measureid_idx + partition_local_table_361378 | partition_local_table_measureid_idx_361378 +(4 rows) + +-- creating a single object should only need to trigger fixing the single object +-- for example, if a partitioned table has already many indexes and we create a new +-- index, only the new index should be fixed +-- create only one shard & one partition so that the output easier to check +SET citus.next_shard_id TO 915000; +SET citus.shard_count TO 1; +SET citus.shard_replication_factor TO 1; +CREATE TABLE parent_table (dist_col int, another_col int, partition_col timestamp, name text) PARTITION BY RANGE (partition_col); +SELECT create_distributed_table('parent_table', 'dist_col'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE p1 PARTITION OF parent_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); +CREATE INDEX i1 ON parent_table(dist_col); +CREATE INDEX i2 ON parent_table(dist_col); +CREATE INDEX i3 ON parent_table(dist_col); +SET citus.log_remote_commands TO ON; +-- only fix i4 +CREATE INDEX i4 ON parent_table(dist_col); +NOTICE: issuing BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;SELECT assign_distributed_transaction_id(xx, xx, 'xxxxxxx'); +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing CREATE INDEX i4_915000 ON fix_idx_names.parent_table_915000 USING btree (dist_col ) +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.i4_915000'::regclass, 'fix_idx_names.p1_915001', 'p1_dist_col_idx3_915001') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing PREPARE TRANSACTION 'citus_xx_xx_xx_xx' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing COMMIT PREPARED 'citus_xx_xx_xx_xx' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +-- only fix the index backing the pkey +ALTER TABLE parent_table ADD CONSTRAINT pkey_cst PRIMARY KEY (dist_col, partition_col); +NOTICE: issuing BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;SELECT assign_distributed_transaction_id(xx, xx, 'xxxxxxx'); +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (915000, 'fix_idx_names', 'ALTER TABLE parent_table ADD CONSTRAINT pkey_cst PRIMARY KEY (dist_col, partition_col);') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.pkey_cst_915000'::regclass, 'fix_idx_names.p1_915001', 'p1_pkey_915001') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing PREPARE TRANSACTION 'citus_xx_xx_xx_xx' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing COMMIT PREPARED 'citus_xx_xx_xx_xx' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +ALTER TABLE parent_table ADD CONSTRAINT unique_cst UNIQUE (dist_col, partition_col); +NOTICE: issuing BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;SELECT assign_distributed_transaction_id(xx, xx, 'xxxxxxx'); +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (915000, 'fix_idx_names', 'ALTER TABLE parent_table ADD CONSTRAINT unique_cst UNIQUE (dist_col, partition_col);') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.unique_cst_915000'::regclass, 'fix_idx_names.p1_915001', 'p1_dist_col_partition_col_key_915001') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing PREPARE TRANSACTION 'citus_xx_xx_xx_xx' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing COMMIT PREPARED 'citus_xx_xx_xx_xx' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +RESET citus.log_remote_commands; +-- we should also be able to alter/drop these indexes +ALTER INDEX i4 RENAME TO i4_renamed; +ALTER INDEX p1_dist_col_idx3 RENAME TO p1_dist_col_idx3_renamed; +ALTER INDEX p1_pkey RENAME TO p1_pkey_renamed; +ALTER INDEX p1_dist_col_partition_col_key RENAME TO p1_dist_col_partition_col_key_renamed; +ALTER INDEX p1_dist_col_idx RENAME TO p1_dist_col_idx_renamed; +-- should be able to create a new partition that is columnar +SET citus.log_remote_commands TO ON; +CREATE TABLE p2(dist_col int NOT NULL, another_col int, partition_col timestamp NOT NULL, name text) USING columnar; +ALTER TABLE parent_table ATTACH PARTITION p2 FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); +NOTICE: issuing SELECT worker_apply_shard_ddl_command (915002, 'fix_idx_names', 'CREATE TABLE fix_idx_names.p2 (dist_col integer NOT NULL, another_col integer, partition_col timestamp without time zone NOT NULL, name text) USING columnar') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT alter_columnar_table_set('fix_idx_names.p2_915002', chunk_group_row_limit => 10000, stripe_row_limit => 150000, compression_level => 3, compression => 'zstd'); +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_shard_ddl_command (915002, 'fix_idx_names', 'ALTER TABLE fix_idx_names.p2 OWNER TO postgres') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;SELECT assign_distributed_transaction_id(xx, xx, 'xxxxxxx'); +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_apply_inter_shard_ddl_command (915000, 'fix_idx_names', 915002, 'fix_idx_names', 'ALTER TABLE parent_table ATTACH PARTITION p2 FOR VALUES FROM (''2019-01-01'') TO (''2020-01-01'');') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.i1_915000'::regclass, 'fix_idx_names.p2_915002', 'p2_dist_col_idx_915002') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.i2_915000'::regclass, 'fix_idx_names.p2_915002', 'p2_dist_col_idx1_915002') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.i3_915000'::regclass, 'fix_idx_names.p2_915002', 'p2_dist_col_idx2_915002') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.i4_renamed_915000'::regclass, 'fix_idx_names.p2_915002', 'p2_dist_col_idx3_915002') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.pkey_cst_915000'::regclass, 'fix_idx_names.p2_915002', 'p2_pkey_915002') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT worker_fix_partition_shard_index_names('fix_idx_names.unique_cst_915000'::regclass, 'fix_idx_names.p2_915002', 'p2_dist_col_partition_col_key_915002') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing PREPARE TRANSACTION 'citus_xx_xx_xx_xx' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing COMMIT PREPARED 'citus_xx_xx_xx_xx' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +RESET citus.log_remote_commands; +DROP INDEX i4_renamed CASCADE; +ALTER TABLE parent_table DROP CONSTRAINT pkey_cst CASCADE; +ALTER TABLE parent_table DROP CONSTRAINT unique_cst CASCADE; DROP SCHEMA fix_idx_names CASCADE; -NOTICE: drop cascades to 5 other objects +NOTICE: drop cascades to 7 other objects DETAIL: drop cascades to table not_partitioned drop cascades to table not_distributed drop cascades to table fk_table -drop cascades to table dist_partitioned_table drop cascades to table p +drop cascades to table date_partitioned_citus_local_table_361377 +drop cascades to table date_partitioned_citus_local_table +drop cascades to table parent_table +SELECT citus_remove_node('localhost', :master_port); + citus_remove_node +--------------------------------------------------------------------- + +(1 row) + SELECT run_command_on_workers($$ DROP SCHEMA IF EXISTS fix_idx_names CASCADE $$); run_command_on_workers --------------------------------------------------------------------- diff --git a/src/test/regress/expected/multi_partitioning.out b/src/test/regress/expected/multi_partitioning.out index 482d41589..3e433cb0b 100644 --- a/src/test/regress/expected/multi_partitioning.out +++ b/src/test/regress/expected/multi_partitioning.out @@ -4275,32 +4275,16 @@ SELECT create_distributed_table('part_table_with_very_long_name', 'dist_col'); CREATE INDEX ON part_table_with_very_long_name USING btree (long_named_integer_col, long_named_part_col); --- shouldn't work -SELECT start_metadata_sync_to_node('localhost', :worker_1_port); -ERROR: relation "part_table_with_long_long_lon_long_named_integer_col_long_n_idx" already exists -CONTEXT: while executing command on localhost:xxxxx -\c - - - :worker_1_port +-- index is created SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'partitioning_schema' AND tablename ilike '%part_table_with_%' ORDER BY 1, 2; - tablename | indexname + tablename | indexname --------------------------------------------------------------------- - part_table_with_long_long_long_long_name_361172 | part_table_with_long_long_lon_long_named_integer_col_long_n_idx - part_table_with_long_long_long_long_name_361174 | part_table_with_long_long_lon_long_named_integer_col_long__idx1 - part_table_with_very_long_name_361168 | part_table_with_very_long_nam_long_named_intege_73d4b078_361168 - part_table_with_very_long_name_361170 | part_table_with_very_long_nam_long_named_intege_73d4b078_361170 -(4 rows) + part_table_with_long_long_long_long_name | part_table_with_long_long_lon_long_named_integer_col_long_n_idx + part_table_with_very_long_name | part_table_with_very_long_nam_long_named_integer_col_long_n_idx +(2 rows) -\c - - - :master_port -SET citus.shard_replication_factor TO 1; -SET search_path = partitioning_schema; --- fix problematic table -SELECT fix_partition_shard_index_names('part_table_with_very_long_name'::regclass); - fix_partition_shard_index_names ---------------------------------------------------------------------- - -(1 row) - --- should work +-- should work properly - no names clashes SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- @@ -4308,7 +4292,7 @@ SELECT start_metadata_sync_to_node('localhost', :worker_1_port); (1 row) \c - - - :worker_1_port --- check that indexes are renamed +-- check that indexes are named properly SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'partitioning_schema' AND tablename ilike '%part_table_with_%' ORDER BY 1, 2; tablename | indexname diff --git a/src/test/regress/expected/partitioned_indexes_create.out b/src/test/regress/expected/partitioned_indexes_create.out new file mode 100644 index 000000000..c3ffd20ba --- /dev/null +++ b/src/test/regress/expected/partitioned_indexes_create.out @@ -0,0 +1,80 @@ +CREATE SCHEMA "partitioned indexes"; +SET search_path TO "partitioned indexes"; +-- test with proper table +CREATE TABLE dist_partitioned_table (dist_col int, another_col int, partition_col timestamp) PARTITION BY RANGE (partition_col); +SELECT create_distributed_table('dist_partitioned_table', 'dist_col'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- create a partition with a long name and another with a short name +CREATE TABLE partition_table_with_very_long_name PARTITION OF dist_partitioned_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); +CREATE TABLE p PARTITION OF dist_partitioned_table FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); +-- create an index on parent table +-- we will see that it doesn't matter whether we name the index on parent or not +-- indexes auto-generated on partitions will not use this name +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE UNIQUE INDEX short ON dist_partitioned_table USING btree (dist_col, partition_col); +-- if we explicitly create index on partition-to-be table, Citus handles the naming +-- hence we would have no broken index names +CREATE TABLE another_partition_table_with_very_long_name (dist_col int, another_col int, partition_col timestamp); +SELECT create_distributed_table('another_partition_table_with_very_long_name', 'dist_col'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE INDEX ON another_partition_table_with_very_long_name USING btree (another_col, partition_col); +-- normally, in arbitrary tests, we DO NOT any of the paramaters, they are managed by the test suite +-- however, due to the issue https://github.com/citusdata/citus/issues/4845 we have to switch to +-- sequential execution on this test. Because this test covers an important case, and the pool size +-- here does not really matter, so we set it to 1 for this transaction block +BEGIN; + SET LOCAL citus.max_adaptive_executor_pool_size TO 1; + ALTER TABLE dist_partitioned_table ATTACH PARTITION another_partition_table_with_very_long_name FOR VALUES FROM ('2020-01-01') TO ('2021-01-01'); +COMMIT; +-- check it works even if we give a weird index name +CREATE TABLE yet_another_partition_table (dist_col int, another_col int, partition_col timestamp); +SELECT create_distributed_table('yet_another_partition_table', 'dist_col'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE INDEX "really weird index name !!" ON yet_another_partition_table USING btree (another_col, partition_col); +ALTER TABLE dist_partitioned_table ATTACH PARTITION yet_another_partition_table FOR VALUES FROM ('2021-01-01') TO ('2022-01-01'); +-- rename & create partition to trigger +-- fix_partition_shard_index_names on an index that a foreign key relies +ALTER INDEX short RENAME TO little_long; +CREATE TABLE p2 PARTITION OF dist_partitioned_table FOR VALUES FROM ('2022-01-01') TO ('2023-01-01'); +-- try creating index to foreign key +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE INDEX ON dist_partitioned_table USING btree (dist_col, partition_col); +-- try with index on only parent +-- this is also an invalid index +-- also try with hash method, not btree +CREATE INDEX short_parent ON ONLY dist_partitioned_table USING hash (dist_col); +-- only another_partition will have the index on dist_col inherited from short_parent +-- hence short_parent will still be invalid +CREATE TABLE another_partition (dist_col int, another_col int, partition_col timestamp); +-- SELECT fix_partition_shard_index_names('another_partition') will be executed +-- automatically at the end of the ATTACH PARTITION command +ALTER TABLE dist_partitioned_table ATTACH PARTITION another_partition FOR VALUES FROM ('2017-01-01') TO ('2018-01-01'); +-- try with expression indexes +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE INDEX expression_index ON dist_partitioned_table ((dist_col || ' ' || another_col)) INCLUDE(another_col) WITH(fillfactor 80) WHERE (dist_col > 10); +ERROR: syntax error at or near "80" +-- try with statistics on index +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE INDEX statistics_on_index on dist_partitioned_table ((dist_col+another_col), (dist_col-another_col)); +ALTER INDEX statistics_on_index ALTER COLUMN 1 SET STATISTICS 3737; +ALTER INDEX statistics_on_index ALTER COLUMN 2 SET STATISTICS 3737; +-- we can drop index on detached partition +-- https://github.com/citusdata/citus/issues/5138 +ALTER TABLE dist_partitioned_table DETACH PARTITION p; +DROP INDEX p_dist_col_partition_col_idx; diff --git a/src/test/regress/expected/partitioning_issue_3970.out b/src/test/regress/expected/partitioning_issue_3970.out index 8d343c726..7072d019a 100644 --- a/src/test/regress/expected/partitioning_issue_3970.out +++ b/src/test/regress/expected/partitioning_issue_3970.out @@ -88,14 +88,14 @@ ORDER BY 1,2,3; part_table_p202008_1690008 | ck_012345678901234567890123456789012345678901234567890123456789 | CHECK (my_seq > 10) part_table_p202008_1690008 | dist_fk_1690000 | FOREIGN KEY (seq) REFERENCES test_3970.dist_1690004(seq) part_table_p202008_1690008 | my_seq | CHECK (my_seq > 0) - part_table_p202008_1690008 | part_table_p202008_1690008_seq_work_ymdt_key | UNIQUE (seq, work_ymdt) part_table_p202008_1690008 | part_table_p202008_pkey_1690008 | PRIMARY KEY (seq, work_ymdt) + part_table_p202008_1690008 | part_table_p202008_seq_work_ymdt_key_1690008 | UNIQUE (seq, work_ymdt) part_table_p202008_1690008 | work_memo_check | CHECK (octet_length(work_memo::text) <= 150) part_table_p202008_1690010 | ck_012345678901234567890123456789012345678901234567890123456789 | CHECK (my_seq > 10) part_table_p202008_1690010 | dist_fk_1690002 | FOREIGN KEY (seq) REFERENCES test_3970.dist_1690006(seq) part_table_p202008_1690010 | my_seq | CHECK (my_seq > 0) - part_table_p202008_1690010 | part_table_p202008_1690010_seq_work_ymdt_key | UNIQUE (seq, work_ymdt) part_table_p202008_1690010 | part_table_p202008_pkey_1690010 | PRIMARY KEY (seq, work_ymdt) + part_table_p202008_1690010 | part_table_p202008_seq_work_ymdt_key_1690010 | UNIQUE (seq, work_ymdt) part_table_p202008_1690010 | work_memo_check | CHECK (octet_length(work_memo::text) <= 150) part_table_p202009_1690012 | ck_012345678901234567890123456789012345678901234567890123456789 | CHECK (my_seq > 10) part_table_p202009_1690012 | dist_fk_1690000 | FOREIGN KEY (seq) REFERENCES test_3970.dist_1690004(seq) diff --git a/src/test/regress/spec/isolation_fix_partition_shard_index_names.spec b/src/test/regress/spec/isolation_fix_partition_shard_index_names.spec index 6c11978f9..88b7df88c 100644 --- a/src/test/regress/spec/isolation_fix_partition_shard_index_names.spec +++ b/src/test/regress/spec/isolation_fix_partition_shard_index_names.spec @@ -1,6 +1,7 @@ setup { CREATE TABLE dist_partitioned_table(a INT, created_at timestamptz) PARTITION BY RANGE (created_at); + CREATE TABLE p PARTITION OF dist_partitioned_table FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); SELECT create_distributed_table('dist_partitioned_table', 'a'); } @@ -16,6 +17,16 @@ step "s1-begin" BEGIN; } +step "s1-select-from-table" +{ + SELECT * FROM dist_partitioned_table; +} + +step "s1-insert-into-table" +{ + INSERT INTO dist_partitioned_table VALUES (0, '2019-01-01'); +} + step "s1-drop-table" { DROP TABLE dist_partitioned_table; @@ -39,6 +50,16 @@ step "s2-fix-partition-shard-index-names" SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); } +step "s2-create-index" +{ + CREATE INDEX ON dist_partitioned_table USING btree(a); +} + +step "s2-create-index-concurrently" +{ + CREATE INDEX CONCURRENTLY ON dist_partitioned_table USING btree(a); +} + step "s2-commit" { COMMIT; @@ -46,3 +67,18 @@ step "s2-commit" permutation "s1-begin" "s1-drop-table" "s2-fix-partition-shard-index-names" "s1-commit" permutation "s2-begin" "s2-fix-partition-shard-index-names" "s1-drop-table" "s2-commit" + +// CREATE INDEX should not block concurrent reads but should block concurrent writes +permutation "s2-begin" "s2-create-index" "s1-select-from-table" "s2-commit" +permutation "s2-begin" "s2-create-index" "s1-insert-into-table" "s2-commit" + +// CREATE INDEX CONCURRENTLY is currently not supported for partitioned tables in PG +// when it's supported, we would want to not block any concurrent reads or writes +permutation "s1-begin" "s1-select-from-table" "s2-create-index-concurrently" "s1-commit" +permutation "s1-begin" "s1-insert-into-table" "s2-create-index-concurrently" "s1-commit" + +// running the following just for consistency +permutation "s1-begin" "s1-select-from-table" "s2-create-index" "s1-commit" +permutation "s1-begin" "s1-insert-into-table" "s2-create-index" "s1-commit" +permutation "s2-begin" "s2-create-index-concurrently" "s1-select-from-table" "s2-commit" +permutation "s2-begin" "s2-create-index-concurrently" "s1-insert-into-table" "s2-commit" diff --git a/src/test/regress/sql/multi_fix_partition_shard_index_names.sql b/src/test/regress/sql/multi_fix_partition_shard_index_names.sql index 0d7c6cffa..24ddeb2a4 100644 --- a/src/test/regress/sql/multi_fix_partition_shard_index_names.sql +++ b/src/test/regress/sql/multi_fix_partition_shard_index_names.sql @@ -37,36 +37,30 @@ CREATE TABLE p PARTITION OF dist_partitioned_table FOR VALUES FROM ('2019-01-01' -- create an index on parent table -- we will see that it doesn't matter whether we name the index on parent or not -- indexes auto-generated on partitions will not use this name +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX short ON dist_partitioned_table USING btree (another_col, partition_col); SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :worker_1_port --- Note that, the shell table from above partition_table_with_very_long_name --- and its shard partition_table_with_very_long_name_910008 --- have the same index name: partition_table_with_very_long_na_another_col_partition_col_idx -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - -\c - - - :master_port --- this should fail because of the name clash explained above -SELECT start_metadata_sync_to_node('localhost', :worker_1_port); - --- let's fix the problematic table -SET search_path TO fix_idx_names, public; -SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); - -\c - - - :worker_1_port +-- the names are generated correctly -- shard id has been appended to all index names which didn't end in shard id -- this goes in line with Citus's way of naming indexes of shards: always append shardid to the end SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :master_port -SET search_path TO fix_idx_names, public; - --- this should now work +-- this should work properly SELECT start_metadata_sync_to_node('localhost', :worker_1_port); --- if we run this command again, the names will not change anymore since shardid is appended to them +\c - - - :worker_1_port +-- we have no clashes +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + +\c - - - :master_port +SET search_path TO fix_idx_names, public; + +-- if we run this command again, the names will not change since shardid is appended to them SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); SELECT fix_all_partition_shard_index_names(); @@ -113,24 +107,20 @@ DROP INDEX short; DROP TABLE yet_another_partition_table, another_partition_table_with_very_long_name; -- this will create constraint1 index on parent SET citus.max_adaptive_executor_pool_size TO 1; +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the ADD CONSTRAINT command ALTER TABLE dist_partitioned_table ADD CONSTRAINT constraint1 UNIQUE (dist_col, partition_col); RESET citus.max_adaptive_executor_pool_size; CREATE TABLE fk_table (id int, fk_column timestamp, FOREIGN KEY (id, fk_column) REFERENCES dist_partitioned_table (dist_col, partition_col)); -- try creating index to foreign key +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX ON dist_partitioned_table USING btree (dist_col, partition_col); SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :worker_1_port --- index names don't end in shardid for partitions -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - -\c - - - :master_port -SET search_path TO fix_idx_names, public; -SELECT fix_all_partition_shard_index_names(); - -\c - - - :worker_1_port --- now index names end in shardid +-- index names end in shardid for partitions SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :master_port @@ -147,6 +137,8 @@ CREATE INDEX short_parent ON ONLY dist_partitioned_table USING hash (dist_col); -- only another_partition will have the index on dist_col inherited from short_parent -- hence short_parent will still be invalid CREATE TABLE another_partition (dist_col int, another_col int, partition_col timestamp); +-- SELECT fix_partition_shard_index_names('another_partition') will be executed +-- automatically at the end of the ATTACH PARTITION command ALTER TABLE dist_partitioned_table ATTACH PARTITION another_partition FOR VALUES FROM ('2017-01-01') TO ('2018-01-01'); SELECT c.relname AS indexname @@ -158,17 +150,7 @@ CREATE INDEX short_child ON ONLY p USING hash (dist_col); SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :worker_1_port --- index names are already correct except for inherited index for another_partition -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - -\c - - - :master_port -SET search_path TO fix_idx_names, public; --- this will fix inherited index for another_partition -SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); --- this will error out becuase p is not partitioned, it is rather a partition -SELECT fix_partition_shard_index_names('p'::regclass); - -\c - - - :worker_1_port +-- index names are already correct, including inherited index for another_partition SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :master_port @@ -178,9 +160,13 @@ DROP INDEX short_parent; DROP INDEX short_child; DROP TABLE another_partition; --- expression indexes have the same problem with naming +-- try with expression indexes +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX expression_index ON dist_partitioned_table ((dist_col || ' ' || another_col)); -- try with statistics on index +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX statistics_on_index on dist_partitioned_table ((dist_col+another_col), (dist_col-another_col)); ALTER INDEX statistics_on_index ALTER COLUMN 1 SET STATISTICS 3737; ALTER INDEX statistics_on_index ALTER COLUMN 2 SET STATISTICS 3737; @@ -188,13 +174,7 @@ ALTER INDEX statistics_on_index ALTER COLUMN 2 SET STATISTICS 3737; SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :worker_1_port -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - -\c - - - :master_port -SET search_path TO fix_idx_names, public; -SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); - -\c - - - :worker_1_port +-- we have correct names SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :master_port @@ -230,19 +210,14 @@ SELECT create_distributed_table('dist_partitioned_table', 'dist_col'); CREATE TABLE partition_table_with_very_long_name PARTITION OF dist_partitioned_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); -- create an index on parent table +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX index_rep_factor_2 ON dist_partitioned_table USING btree (another_col, partition_col); SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; \c - - - :worker_2_port -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - -\c - - - :master_port --- let's fix the problematic table -SET search_path TO fix_idx_names, public; -SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); - -\c - - - :worker_2_port +-- index names are correct -- shard id has been appended to all index names which didn't end in shard id -- this goes in line with Citus's way of naming indexes of shards: always append shardid to the end SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; @@ -264,11 +239,10 @@ RESET ROLE; SET search_path TO fix_idx_names, public; DROP TABLE dist_partitioned_table; --- also, we cannot do any further operations (e.g. rename) on the indexes of partitions because --- the index names on shards of partitions have been generated by Postgres, not Citus --- it doesn't matter here whether the partition name is long or short - +-- We can do any further operations (e.g. rename) on the indexes of partitions because +-- the index names on shards of partitions have Citus naming, hence are reachable -- replicate scenario from above but this time with one shard so that this test isn't flaky + SET citus.shard_count TO 1; SET citus.shard_replication_factor TO 1; SET citus.next_shard_id TO 910030; @@ -277,41 +251,18 @@ CREATE TABLE dist_partitioned_table (dist_col int, another_col int, partition_co SELECT create_distributed_table('dist_partitioned_table', 'dist_col'); CREATE TABLE partition_table_with_very_long_name PARTITION OF dist_partitioned_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); CREATE TABLE p PARTITION OF dist_partitioned_table FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command CREATE INDEX short ON dist_partitioned_table USING btree (another_col, partition_col); --- rename shouldn't work +-- rename works! ALTER INDEX partition_table_with_very_long_na_another_col_partition_col_idx RENAME TO partition_table_with_very_long_name_idx; --- we currently can't drop index on detached partition +-- we can drop index on detached partition -- https://github.com/citusdata/citus/issues/5138 ALTER TABLE dist_partitioned_table DETACH PARTITION p; DROP INDEX p_another_col_partition_col_idx; --- let's reattach and retry after fixing index names -ALTER TABLE dist_partitioned_table ATTACH PARTITION p FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); - -\c - - - :worker_1_port --- check the current broken index names -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - -\c - - - :master_port -SET search_path TO fix_idx_names, public; --- fix index names -SELECT fix_all_partition_shard_index_names(); - -\c - - - :worker_1_port --- check the fixed index names -SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; - -\c - - - :master_port -SET search_path TO fix_idx_names, public; --- should now work -ALTER INDEX partition_table_with_very_long_na_another_col_partition_col_idx RENAME TO partition_table_with_very_long_name_idx; - --- now we can drop index on detached partition -ALTER TABLE dist_partitioned_table DETACH PARTITION p; -DROP INDEX p_another_col_partition_col_idx; - \c - - - :worker_1_port -- check that indexes have been renamed -- and that index on p has been dropped (it won't appear) @@ -319,6 +270,74 @@ SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' O \c - - - :master_port SET search_path TO fix_idx_names, public; +DROP TABLE dist_partitioned_table; + +-- test with citus local table +SET client_min_messages TO WARNING; +SELECT 1 FROM citus_add_node('localhost', :master_port, groupid=>0); +RESET client_min_messages; + +CREATE TABLE date_partitioned_citus_local_table( + measureid integer, + eventdate date, + measure_data jsonb) PARTITION BY RANGE(eventdate); +SELECT citus_add_local_table_to_metadata('date_partitioned_citus_local_table'); + +CREATE TABLE partition_local_table PARTITION OF date_partitioned_citus_local_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); + +-- SELECT fix_partition_shard_index_names('date_partitioned_citus_local_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE INDEX ON date_partitioned_citus_local_table USING btree(measureid); + +-- check that index names are correct +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + + +-- creating a single object should only need to trigger fixing the single object +-- for example, if a partitioned table has already many indexes and we create a new +-- index, only the new index should be fixed + +-- create only one shard & one partition so that the output easier to check +SET citus.next_shard_id TO 915000; + +SET citus.shard_count TO 1; +SET citus.shard_replication_factor TO 1; +CREATE TABLE parent_table (dist_col int, another_col int, partition_col timestamp, name text) PARTITION BY RANGE (partition_col); +SELECT create_distributed_table('parent_table', 'dist_col'); +CREATE TABLE p1 PARTITION OF parent_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); + +CREATE INDEX i1 ON parent_table(dist_col); +CREATE INDEX i2 ON parent_table(dist_col); +CREATE INDEX i3 ON parent_table(dist_col); + +SET citus.log_remote_commands TO ON; + +-- only fix i4 +CREATE INDEX i4 ON parent_table(dist_col); + +-- only fix the index backing the pkey +ALTER TABLE parent_table ADD CONSTRAINT pkey_cst PRIMARY KEY (dist_col, partition_col); +ALTER TABLE parent_table ADD CONSTRAINT unique_cst UNIQUE (dist_col, partition_col); + +RESET citus.log_remote_commands; + +-- we should also be able to alter/drop these indexes +ALTER INDEX i4 RENAME TO i4_renamed; +ALTER INDEX p1_dist_col_idx3 RENAME TO p1_dist_col_idx3_renamed; +ALTER INDEX p1_pkey RENAME TO p1_pkey_renamed; +ALTER INDEX p1_dist_col_partition_col_key RENAME TO p1_dist_col_partition_col_key_renamed; +ALTER INDEX p1_dist_col_idx RENAME TO p1_dist_col_idx_renamed; + +-- should be able to create a new partition that is columnar +SET citus.log_remote_commands TO ON; +CREATE TABLE p2(dist_col int NOT NULL, another_col int, partition_col timestamp NOT NULL, name text) USING columnar; +ALTER TABLE parent_table ATTACH PARTITION p2 FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); +RESET citus.log_remote_commands; + +DROP INDEX i4_renamed CASCADE; +ALTER TABLE parent_table DROP CONSTRAINT pkey_cst CASCADE; +ALTER TABLE parent_table DROP CONSTRAINT unique_cst CASCADE; DROP SCHEMA fix_idx_names CASCADE; +SELECT citus_remove_node('localhost', :master_port); SELECT run_command_on_workers($$ DROP SCHEMA IF EXISTS fix_idx_names CASCADE $$); diff --git a/src/test/regress/sql/multi_partitioning.sql b/src/test/regress/sql/multi_partitioning.sql index 702ffbe23..eaf614fae 100644 --- a/src/test/regress/sql/multi_partitioning.sql +++ b/src/test/regress/sql/multi_partitioning.sql @@ -1986,23 +1986,15 @@ SELECT create_distributed_table('part_table_with_very_long_name', 'dist_col'); CREATE INDEX ON part_table_with_very_long_name USING btree (long_named_integer_col, long_named_part_col); --- shouldn't work -SELECT start_metadata_sync_to_node('localhost', :worker_1_port); - -\c - - - :worker_1_port +-- index is created SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'partitioning_schema' AND tablename ilike '%part_table_with_%' ORDER BY 1, 2; -\c - - - :master_port -SET citus.shard_replication_factor TO 1; -SET search_path = partitioning_schema; --- fix problematic table -SELECT fix_partition_shard_index_names('part_table_with_very_long_name'::regclass); --- should work +-- should work properly - no names clashes SELECT start_metadata_sync_to_node('localhost', :worker_1_port); \c - - - :worker_1_port --- check that indexes are renamed +-- check that indexes are named properly SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'partitioning_schema' AND tablename ilike '%part_table_with_%' ORDER BY 1, 2; diff --git a/src/test/regress/sql/partitioned_indexes_create.sql b/src/test/regress/sql/partitioned_indexes_create.sql new file mode 100644 index 000000000..dcb2ab37a --- /dev/null +++ b/src/test/regress/sql/partitioned_indexes_create.sql @@ -0,0 +1,76 @@ +CREATE SCHEMA "partitioned indexes"; +SET search_path TO "partitioned indexes"; + +-- test with proper table +CREATE TABLE dist_partitioned_table (dist_col int, another_col int, partition_col timestamp) PARTITION BY RANGE (partition_col); +SELECT create_distributed_table('dist_partitioned_table', 'dist_col'); + +-- create a partition with a long name and another with a short name +CREATE TABLE partition_table_with_very_long_name PARTITION OF dist_partitioned_table FOR VALUES FROM ('2018-01-01') TO ('2019-01-01'); +CREATE TABLE p PARTITION OF dist_partitioned_table FOR VALUES FROM ('2019-01-01') TO ('2020-01-01'); + +-- create an index on parent table +-- we will see that it doesn't matter whether we name the index on parent or not +-- indexes auto-generated on partitions will not use this name +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE UNIQUE INDEX short ON dist_partitioned_table USING btree (dist_col, partition_col); + +-- if we explicitly create index on partition-to-be table, Citus handles the naming +-- hence we would have no broken index names +CREATE TABLE another_partition_table_with_very_long_name (dist_col int, another_col int, partition_col timestamp); +SELECT create_distributed_table('another_partition_table_with_very_long_name', 'dist_col'); +CREATE INDEX ON another_partition_table_with_very_long_name USING btree (another_col, partition_col); + +-- normally, in arbitrary tests, we DO NOT any of the paramaters, they are managed by the test suite +-- however, due to the issue https://github.com/citusdata/citus/issues/4845 we have to switch to +-- sequential execution on this test. Because this test covers an important case, and the pool size +-- here does not really matter, so we set it to 1 for this transaction block +BEGIN; + SET LOCAL citus.max_adaptive_executor_pool_size TO 1; + ALTER TABLE dist_partitioned_table ATTACH PARTITION another_partition_table_with_very_long_name FOR VALUES FROM ('2020-01-01') TO ('2021-01-01'); +COMMIT; + +-- check it works even if we give a weird index name +CREATE TABLE yet_another_partition_table (dist_col int, another_col int, partition_col timestamp); +SELECT create_distributed_table('yet_another_partition_table', 'dist_col'); +CREATE INDEX "really weird index name !!" ON yet_another_partition_table USING btree (another_col, partition_col); +ALTER TABLE dist_partitioned_table ATTACH PARTITION yet_another_partition_table FOR VALUES FROM ('2021-01-01') TO ('2022-01-01'); + +-- rename & create partition to trigger +-- fix_partition_shard_index_names on an index that a foreign key relies +ALTER INDEX short RENAME TO little_long; +CREATE TABLE p2 PARTITION OF dist_partitioned_table FOR VALUES FROM ('2022-01-01') TO ('2023-01-01'); + + +-- try creating index to foreign key +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE INDEX ON dist_partitioned_table USING btree (dist_col, partition_col); + +-- try with index on only parent +-- this is also an invalid index +-- also try with hash method, not btree +CREATE INDEX short_parent ON ONLY dist_partitioned_table USING hash (dist_col); +-- only another_partition will have the index on dist_col inherited from short_parent +-- hence short_parent will still be invalid +CREATE TABLE another_partition (dist_col int, another_col int, partition_col timestamp); +-- SELECT fix_partition_shard_index_names('another_partition') will be executed +-- automatically at the end of the ATTACH PARTITION command +ALTER TABLE dist_partitioned_table ATTACH PARTITION another_partition FOR VALUES FROM ('2017-01-01') TO ('2018-01-01'); + +-- try with expression indexes +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE INDEX expression_index ON dist_partitioned_table ((dist_col || ' ' || another_col)) INCLUDE(another_col) WITH(fillfactor 80) WHERE (dist_col > 10); +-- try with statistics on index +-- SELECT fix_partition_shard_index_names('dist_partitioned_table') will be executed +-- automatically at the end of the CREATE INDEX command +CREATE INDEX statistics_on_index on dist_partitioned_table ((dist_col+another_col), (dist_col-another_col)); +ALTER INDEX statistics_on_index ALTER COLUMN 1 SET STATISTICS 3737; +ALTER INDEX statistics_on_index ALTER COLUMN 2 SET STATISTICS 3737; + +-- we can drop index on detached partition +-- https://github.com/citusdata/citus/issues/5138 +ALTER TABLE dist_partitioned_table DETACH PARTITION p; +DROP INDEX p_dist_col_partition_col_idx;