From d0390af72d1b19d297289b51064310eacc2a2d46 Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Thu, 7 Oct 2021 19:34:52 +0300 Subject: [PATCH] Add fix_partition_shard_index_names udf to fix currently broken names (#5291) * Add udf to include shardId in broken partition shard index names * Address reviews: rename index such that operations can be done on it * More comprehensive index tests * Final touches and formatting --- .../distributed/sql/citus--10.2-3--11.0-1.sql | 4 + .../sql/downgrades/citus--11.0-1--10.2-3.sql | 4 + .../11.0-1.sql | 21 + .../latest.sql | 21 + .../11.0-1.sql | 6 + .../latest.sql | 6 + .../11.0-1.sql | 10 + .../latest.sql | 10 + .../utils/multi_partitioning_utils.c | 371 ++++++++++ ...lation_fix_partition_shard_index_names.out | 54 ++ src/test/regress/expected/multi_extension.out | 7 +- .../multi_fix_partition_shard_index_names.out | 638 ++++++++++++++++++ .../regress/expected/multi_partitioning.out | 81 ++- .../expected/upgrade_list_citus_objects.out | 5 +- src/test/regress/isolation_schedule | 1 + src/test/regress/multi_1_schedule | 1 + ...ation_fix_partition_shard_index_names.spec | 48 ++ .../multi_fix_partition_shard_index_names.sql | 321 +++++++++ src/test/regress/sql/multi_partitioning.sql | 40 ++ 19 files changed, 1642 insertions(+), 7 deletions(-) create mode 100644 src/backend/distributed/sql/udfs/fix_all_partition_shard_index_names/11.0-1.sql create mode 100644 src/backend/distributed/sql/udfs/fix_all_partition_shard_index_names/latest.sql create mode 100644 src/backend/distributed/sql/udfs/fix_partition_shard_index_names/11.0-1.sql create mode 100644 src/backend/distributed/sql/udfs/fix_partition_shard_index_names/latest.sql create mode 100644 src/backend/distributed/sql/udfs/worker_fix_partition_shard_index_names/11.0-1.sql create mode 100644 src/backend/distributed/sql/udfs/worker_fix_partition_shard_index_names/latest.sql create mode 100644 src/test/regress/expected/isolation_fix_partition_shard_index_names.out create mode 100644 src/test/regress/expected/multi_fix_partition_shard_index_names.out create mode 100644 src/test/regress/spec/isolation_fix_partition_shard_index_names.spec create mode 100644 src/test/regress/sql/multi_fix_partition_shard_index_names.sql diff --git a/src/backend/distributed/sql/citus--10.2-3--11.0-1.sql b/src/backend/distributed/sql/citus--10.2-3--11.0-1.sql index 322b76570..3142ea088 100644 --- a/src/backend/distributed/sql/citus--10.2-3--11.0-1.sql +++ b/src/backend/distributed/sql/citus--10.2-3--11.0-1.sql @@ -1,3 +1,7 @@ -- citus--10.2-3--11.0-1 -- bump version to 11.0-1 + +#include "udfs/fix_partition_shard_index_names/11.0-1.sql" +#include "udfs/fix_all_partition_shard_index_names/11.0-1.sql" +#include "udfs/worker_fix_partition_shard_index_names/11.0-1.sql" diff --git a/src/backend/distributed/sql/downgrades/citus--11.0-1--10.2-3.sql b/src/backend/distributed/sql/downgrades/citus--11.0-1--10.2-3.sql index 34485a80a..10d0588c4 100644 --- a/src/backend/distributed/sql/downgrades/citus--11.0-1--10.2-3.sql +++ b/src/backend/distributed/sql/downgrades/citus--11.0-1--10.2-3.sql @@ -1 +1,5 @@ -- citus--11.0-1--10.2-3 + +DROP FUNCTION pg_catalog.fix_all_partition_shard_index_names(); +DROP FUNCTION pg_catalog.fix_partition_shard_index_names(regclass); +DROP FUNCTION pg_catalog.worker_fix_partition_shard_index_names(regclass, text, text); diff --git a/src/backend/distributed/sql/udfs/fix_all_partition_shard_index_names/11.0-1.sql b/src/backend/distributed/sql/udfs/fix_all_partition_shard_index_names/11.0-1.sql new file mode 100644 index 000000000..debb5825c --- /dev/null +++ b/src/backend/distributed/sql/udfs/fix_all_partition_shard_index_names/11.0-1.sql @@ -0,0 +1,21 @@ +CREATE OR REPLACE FUNCTION pg_catalog.fix_all_partition_shard_index_names() + RETURNS SETOF regclass + LANGUAGE plpgsql + AS $$ +DECLARE + dist_partitioned_table_name regclass; +BEGIN + FOR dist_partitioned_table_name IN SELECT p.logicalrelid + FROM pg_dist_partition p + JOIN pg_class c ON p.logicalrelid = c.oid + WHERE c.relkind = 'p' + ORDER BY c.relname, c.oid + LOOP + EXECUTE 'SELECT fix_partition_shard_index_names( ' || quote_literal(dist_partitioned_table_name) || ' )'; + RETURN NEXT dist_partitioned_table_name; + END LOOP; + RETURN; +END; +$$; +COMMENT ON FUNCTION pg_catalog.fix_all_partition_shard_index_names() + IS 'fix index names on partition shards of all tables'; diff --git a/src/backend/distributed/sql/udfs/fix_all_partition_shard_index_names/latest.sql b/src/backend/distributed/sql/udfs/fix_all_partition_shard_index_names/latest.sql new file mode 100644 index 000000000..debb5825c --- /dev/null +++ b/src/backend/distributed/sql/udfs/fix_all_partition_shard_index_names/latest.sql @@ -0,0 +1,21 @@ +CREATE OR REPLACE FUNCTION pg_catalog.fix_all_partition_shard_index_names() + RETURNS SETOF regclass + LANGUAGE plpgsql + AS $$ +DECLARE + dist_partitioned_table_name regclass; +BEGIN + FOR dist_partitioned_table_name IN SELECT p.logicalrelid + FROM pg_dist_partition p + JOIN pg_class c ON p.logicalrelid = c.oid + WHERE c.relkind = 'p' + ORDER BY c.relname, c.oid + LOOP + EXECUTE 'SELECT fix_partition_shard_index_names( ' || quote_literal(dist_partitioned_table_name) || ' )'; + RETURN NEXT dist_partitioned_table_name; + END LOOP; + RETURN; +END; +$$; +COMMENT ON FUNCTION pg_catalog.fix_all_partition_shard_index_names() + IS 'fix index names on partition shards of all tables'; diff --git a/src/backend/distributed/sql/udfs/fix_partition_shard_index_names/11.0-1.sql b/src/backend/distributed/sql/udfs/fix_partition_shard_index_names/11.0-1.sql new file mode 100644 index 000000000..fe0cadfa0 --- /dev/null +++ b/src/backend/distributed/sql/udfs/fix_partition_shard_index_names/11.0-1.sql @@ -0,0 +1,6 @@ +CREATE FUNCTION pg_catalog.fix_partition_shard_index_names(table_name regclass) + RETURNS void + LANGUAGE C STRICT + AS 'MODULE_PATHNAME', $$fix_partition_shard_index_names$$; +COMMENT ON FUNCTION pg_catalog.fix_partition_shard_index_names(table_name regclass) + IS 'fix index names on partition shards of given table'; diff --git a/src/backend/distributed/sql/udfs/fix_partition_shard_index_names/latest.sql b/src/backend/distributed/sql/udfs/fix_partition_shard_index_names/latest.sql new file mode 100644 index 000000000..fe0cadfa0 --- /dev/null +++ b/src/backend/distributed/sql/udfs/fix_partition_shard_index_names/latest.sql @@ -0,0 +1,6 @@ +CREATE FUNCTION pg_catalog.fix_partition_shard_index_names(table_name regclass) + RETURNS void + LANGUAGE C STRICT + AS 'MODULE_PATHNAME', $$fix_partition_shard_index_names$$; +COMMENT ON FUNCTION pg_catalog.fix_partition_shard_index_names(table_name regclass) + IS 'fix index names on partition shards of given table'; diff --git a/src/backend/distributed/sql/udfs/worker_fix_partition_shard_index_names/11.0-1.sql b/src/backend/distributed/sql/udfs/worker_fix_partition_shard_index_names/11.0-1.sql new file mode 100644 index 000000000..830de5c80 --- /dev/null +++ b/src/backend/distributed/sql/udfs/worker_fix_partition_shard_index_names/11.0-1.sql @@ -0,0 +1,10 @@ +CREATE FUNCTION pg_catalog.worker_fix_partition_shard_index_names(parent_shard_index regclass, + partition_shard text, + new_partition_shard_index_name text) + RETURNS void + LANGUAGE C STRICT + AS 'MODULE_PATHNAME', $$worker_fix_partition_shard_index_names$$; +COMMENT ON FUNCTION pg_catalog.worker_fix_partition_shard_index_names(parent_shard_index regclass, + partition_shard text, + new_partition_shard_index_name text) + IS 'fix the name of the index on given partition shard that is child of given parent_index'; diff --git a/src/backend/distributed/sql/udfs/worker_fix_partition_shard_index_names/latest.sql b/src/backend/distributed/sql/udfs/worker_fix_partition_shard_index_names/latest.sql new file mode 100644 index 000000000..830de5c80 --- /dev/null +++ b/src/backend/distributed/sql/udfs/worker_fix_partition_shard_index_names/latest.sql @@ -0,0 +1,10 @@ +CREATE FUNCTION pg_catalog.worker_fix_partition_shard_index_names(parent_shard_index regclass, + partition_shard text, + new_partition_shard_index_name text) + RETURNS void + LANGUAGE C STRICT + AS 'MODULE_PATHNAME', $$worker_fix_partition_shard_index_names$$; +COMMENT ON FUNCTION pg_catalog.worker_fix_partition_shard_index_names(parent_shard_index regclass, + partition_shard text, + new_partition_shard_index_name text) + IS 'fix the name of the index on given partition shard that is child of given parent_index'; diff --git a/src/backend/distributed/utils/multi_partitioning_utils.c b/src/backend/distributed/utils/multi_partitioning_utils.c index 50d77a84a..34737da27 100644 --- a/src/backend/distributed/utils/multi_partitioning_utils.c +++ b/src/backend/distributed/utils/multi_partitioning_utils.c @@ -11,11 +11,13 @@ #include "access/genam.h" #include "access/heapam.h" #include "access/htup_details.h" +#include "catalog/index.h" #include "catalog/indexing.h" #include "catalog/partition.h" #include "catalog/pg_class.h" #include "catalog/pg_constraint.h" #include "catalog/pg_inherits.h" +#include "commands/tablecmds.h" #include "common/string.h" #include "distributed/citus_nodes.h" #include "distributed/adaptive_executor.h" @@ -26,13 +28,16 @@ #include "distributed/deparse_shard_query.h" #include "distributed/listutils.h" #include "distributed/metadata_utility.h" +#include "distributed/multi_executor.h" #include "distributed/multi_partitioning_utils.h" #include "distributed/multi_physical_planner.h" #include "distributed/relay_utility.h" #include "distributed/resource_lock.h" #include "distributed/shardinterval_utils.h" #include "distributed/version_compat.h" +#include "distributed/worker_protocol.h" #include "lib/stringinfo.h" +#include "nodes/makefuncs.h" #include "nodes/pg_list.h" #include "pgstat.h" #include "partitioning/partdesc.h" @@ -41,12 +46,22 @@ #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/syscache.h" +#include "utils/varlena.h" static char * PartitionBound(Oid partitionId); 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 * WorkerFixPartitionShardIndexNamesCommandList(uint64 parentShardId, + List *indexIdList); +static List * WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex( + char *qualifiedParentShardIndexName, Oid parentIndexId); +static List * WorkerFixPartitionShardIndexNamesCommandListForPartitionIndex(Oid + partitionIndexId, + char * + qualifiedParentShardIndexName); static List * CheckConstraintNameListForRelation(Oid relationId); static bool RelationHasConstraint(Oid relationId, char *constraintName); static char * RenameConstraintCommand(Oid relationId, char *constraintName, @@ -55,6 +70,8 @@ static char * RenameConstraintCommand(Oid relationId, char *constraintName, PG_FUNCTION_INFO_V1(fix_pre_citus10_partitioned_table_constraint_names); PG_FUNCTION_INFO_V1(worker_fix_pre_citus10_partitioned_table_constraint_names); +PG_FUNCTION_INFO_V1(fix_partition_shard_index_names); +PG_FUNCTION_INFO_V1(worker_fix_partition_shard_index_names); /* @@ -130,6 +147,167 @@ 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. + * + * 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, + * and also in the workers. Actually, Postgres auto-generates index names when auto-creating + * indexes on each partition shard of the parent shards. If index name is too long, it + * truncates the name and adds _idx postfix to it. However, when truncating the name, the + * shardId of the partition shard can be lost. This may result in the same index name used for + * the partition shell table and one of the partition shards. + * For more details, check issue #4962 https://github.com/citusdata/citus/issues/4962 + * + * 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: + * generate qualifiedParentShardIndexName -> parentShardIndex + * foreach inheritedPartitionIndex on parentIndex: + * get table relation of inheritedPartitionIndex -> partitionId + * foreach partitionShard in shardListOfPartitionid: + * generate qualifiedPartitionShardName -> partitionShard + * generate newPartitionShardIndexName + * (the following happens in the worker node) + * foreach inheritedPartitionShardIndex on parentShardIndex: + * if table relation of inheritedPartitionShardIndex is partitionShard: + * if inheritedPartitionShardIndex does not have proper name: + * Rename(inheritedPartitionShardIndex, newPartitionShardIndexName) + * break + */ +Datum +fix_partition_shard_index_names(PG_FUNCTION_ARGS) +{ + CheckCitusVersion(ERROR); + 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)))); + } + + if (!IsCitusTable(relationId)) + { + relation_close(relation, NoLock); + ereport(ERROR, (errmsg("fix_partition_shard_index_names can " + "only be called for distributed partitioned 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); + + PG_RETURN_VOID(); +} + + +/* + * worker_fix_partition_shard_index_names fixes the index name of the index on given + * partition shard that has parent the given parent index. + * The parent index should be the index of a shard of a distributed partitioned table. + */ +Datum +worker_fix_partition_shard_index_names(PG_FUNCTION_ARGS) +{ + Oid parentShardIndexId = PG_GETARG_OID(0); + + text *partitionShardName = PG_GETARG_TEXT_P(1); + + /* resolve partitionShardId from passed in schema and partition shard name */ + List *partitionShardNameList = textToQualifiedNameList(partitionShardName); + RangeVar *partitionShard = makeRangeVarFromNameList(partitionShardNameList); + + /* lock the relation with the lock mode */ + bool missing_ok = true; + Oid partitionShardId = RangeVarGetRelid(partitionShard, NoLock, missing_ok); + + if (!OidIsValid(partitionShardId)) + { + PG_RETURN_VOID(); + } + + CheckCitusVersion(ERROR); + EnsureTableOwner(partitionShardId); + + text *newPartitionShardIndexNameText = PG_GETARG_TEXT_P(2); + char *newPartitionShardIndexName = text_to_cstring( + newPartitionShardIndexNameText); + + if (!has_subclass(parentShardIndexId)) + { + ereport(ERROR, (errmsg("could not fix child index names: " + "index is not partitioned"))); + } + + List *partitionShardIndexIds = find_inheritance_children(parentShardIndexId, + ShareRowExclusiveLock); + Oid partitionShardIndexId = InvalidOid; + foreach_oid(partitionShardIndexId, partitionShardIndexIds) + { + if (IndexGetRelation(partitionShardIndexId, false) == partitionShardId) + { + char *partitionShardIndexName = get_rel_name(partitionShardIndexId); + if (ExtractShardIdFromTableName(partitionShardIndexName, missing_ok) == + INVALID_SHARD_ID) + { + /* + * ExtractShardIdFromTableName will return INVALID_SHARD_ID if + * partitionShardIndexName doesn't end in _shardid. In that case, + * we want to rename this partition shard index to newPartitionShardIndexName, + * which ends in _shardid, hence we maintain naming consistency: + * we can reach this partition shard index by conventional Citus naming + */ + RenameStmt *stmt = makeNode(RenameStmt); + + stmt->renameType = OBJECT_INDEX; + stmt->missing_ok = false; + char *idxNamespace = get_namespace_name(get_rel_namespace( + partitionShardIndexId)); + stmt->relation = makeRangeVar(idxNamespace, partitionShardIndexName, -1); + stmt->newname = newPartitionShardIndexName; + + RenameRelation(stmt); + } + break; + } + } + + PG_RETURN_VOID(); +} + + /* * CreateFixPartitionConstraintsTaskList goes over all the partitions of a distributed * partitioned table, and creates the list of tasks to execute @@ -257,6 +435,199 @@ 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. + * + * We create parent_table_shard_count tasks, + * each task with parent_indexes_count x parent_partitions_count query strings. + */ +static List * +CreateFixPartitionShardIndexNamesTaskList(Oid parentRelationId) +{ + List *taskList = NIL; + + /* enumerate the tasks when putting them to the taskList */ + int taskId = 1; + + Relation parentRelation = RelationIdGetRelation(parentRelationId); + + List *parentIndexIdList = RelationGetIndexList(parentRelation); + + /* early exit if the parent relation does not have any indexes */ + if (parentIndexIdList == NIL) + { + RelationClose(parentRelation); + return NIL; + } + + List *partitionList = PartitionList(parentRelationId); + + /* early exit if the parent relation does not have any partitions */ + if (partitionList == NIL) + { + RelationClose(parentRelation); + return NIL; + } + + 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); + } + + ShardInterval *parentShardInterval = NULL; + foreach_ptr(parentShardInterval, parentShardIntervalList) + { + uint64 parentShardId = parentShardInterval->shardId; + + List *queryStringList = WorkerFixPartitionShardIndexNamesCommandList( + parentShardId, parentIndexIdList); + + 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); + + taskList = lappend(taskList, task); + } + + RelationClose(parentRelation); + + return taskList; +} + + +/* + * WorkerFixPartitionShardIndexNamesCommandList creates a list of queries that will fix + * all child index names of parent indexes on given shard of parent partitioned table. + */ +static List * +WorkerFixPartitionShardIndexNamesCommandList(uint64 parentShardId, + List *parentIndexIdList) +{ + List *commandList = NIL; + Oid parentIndexId = InvalidOid; + foreach_oid(parentIndexId, parentIndexIdList) + { + if (!has_subclass(parentIndexId)) + { + continue; + } + + /* + * Get the qualified name of the corresponding index of given parent index + * in the parent shard with given parentShardId + */ + char *parentIndexName = get_rel_name(parentIndexId); + char *parentShardIndexName = pstrdup(parentIndexName); + AppendShardIdToName(&parentShardIndexName, parentShardId); + Oid schemaId = get_rel_namespace(parentIndexId); + char *schemaName = get_namespace_name(schemaId); + char *qualifiedParentShardIndexName = quote_qualified_identifier(schemaName, + parentShardIndexName); + List *commands = WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex( + qualifiedParentShardIndexName, parentIndexId); + commandList = list_concat(commandList, commands); + } + + return commandList; +} + + +/* + * WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex creates a list of queries that will fix + * all child index names of given index on shard of parent partitioned table. + */ +static List * +WorkerFixPartitionShardIndexNamesCommandListForParentShardIndex( + char *qualifiedParentShardIndexName, Oid parentIndexId) +{ + List *commandList = NIL; + + /* + * Get the list of all partition indexes that are children of current + * index on parent + */ + List *partitionIndexIds = find_inheritance_children(parentIndexId, + ShareRowExclusiveLock); + Oid partitionIndexId = InvalidOid; + foreach_oid(partitionIndexId, partitionIndexIds) + { + List *commands = WorkerFixPartitionShardIndexNamesCommandListForPartitionIndex( + partitionIndexId, qualifiedParentShardIndexName); + commandList = list_concat(commandList, commands); + } + return commandList; +} + + +/* + * 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 + */ +static List * +WorkerFixPartitionShardIndexNamesCommandListForPartitionIndex(Oid partitionIndexId, + char * + qualifiedParentShardIndexName) +{ + 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); + + ShardInterval *partitionShardInterval = NULL; + foreach_ptr(partitionShardInterval, partitionShardIntervalList) + { + /* + * Prepare commands for each shard of current partition + * to fix the index name that corresponds to the + * current parent index name + */ + uint64 partitionShardId = partitionShardInterval->shardId; + + /* get qualified partition shard name */ + char *partitionShardName = pstrdup(partitionName); + AppendShardIdToName(&partitionShardName, partitionShardId); + char *qualifiedPartitionShardName = quote_qualified_identifier( + partitionSchemaName, + partitionShardName); + + /* generate the new correct index name */ + char *newPartitionShardIndexName = pstrdup(partitionIndexName); + AppendShardIdToName(&newPartitionShardIndexName, partitionShardId); + + /* create worker_fix_partition_shard_index_names command */ + StringInfo shardQueryString = makeStringInfo(); + appendStringInfo(shardQueryString, + "SELECT worker_fix_partition_shard_index_names(%s::regclass, %s, %s)", + quote_literal_cstr(qualifiedParentShardIndexName), + quote_literal_cstr(qualifiedPartitionShardName), + quote_literal_cstr(newPartitionShardIndexName)); + commandList = lappend(commandList, shardQueryString->data); + } + + return commandList; +} + + /* * RelationHasConstraint checks if a relation has a constraint with a given name. */ 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 new file mode 100644 index 000000000..9a7959386 --- /dev/null +++ b/src/test/regress/expected/isolation_fix_partition_shard_index_names.out @@ -0,0 +1,54 @@ +Parsed test spec with 2 sessions + +starting permutation: s1-begin s1-drop-table s2-fix-partition-shard-index-names s1-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s1-begin: + BEGIN; + +step s1-drop-table: + DROP TABLE dist_partitioned_table; + +step s2-fix-partition-shard-index-names: + SET client_min_messages TO NOTICE; + SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); + +step s1-commit: + COMMIT; + +step s2-fix-partition-shard-index-names: <... completed> +s2: NOTICE: relation with OID XXXX does not exist, skipping +fix_partition_shard_index_names +--------------------------------------------------------------------- + +(1 row) + + +starting permutation: s2-begin s2-fix-partition-shard-index-names s1-drop-table s2-commit +create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +step s2-begin: + BEGIN; + +step s2-fix-partition-shard-index-names: + SET client_min_messages TO NOTICE; + SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); + +fix_partition_shard_index_names +--------------------------------------------------------------------- + +(1 row) + +step s1-drop-table: + DROP TABLE dist_partitioned_table; + +step s2-commit: + COMMIT; + +step s1-drop-table: <... completed> diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 06923a0ad..3c9328320 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -892,9 +892,12 @@ SELECT * FROM multi_extension.print_extension_changes(); -- Snapshot of state at 11.0-1 ALTER EXTENSION citus UPDATE TO '11.0-1'; SELECT * FROM multi_extension.print_extension_changes(); - previous_object | current_object + previous_object | current_object --------------------------------------------------------------------- -(0 rows) + | function fix_all_partition_shard_index_names() SETOF regclass + | function fix_partition_shard_index_names(regclass) void + | function worker_fix_partition_shard_index_names(regclass,text,text) void +(3 rows) DROP TABLE multi_extension.prev_objects, multi_extension.extension_diff; -- show running version 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 new file mode 100644 index 000000000..deaed1eeb --- /dev/null +++ b/src/test/regress/expected/multi_fix_partition_shard_index_names.out @@ -0,0 +1,638 @@ +--------------------------------------------------------------------- +-- multi_fix_partition_shard_index_names +-- check the following two issues +-- https://github.com/citusdata/citus/issues/4962 +-- https://github.com/citusdata/citus/issues/5138 +--------------------------------------------------------------------- +SET citus.next_shard_id TO 910000; +SET citus.shard_replication_factor TO 1; +CREATE SCHEMA fix_idx_names; +SET search_path TO fix_idx_names, public; +-- NULL input should automatically return NULL since +-- fix_partition_shard_index_names is strict +-- same for worker_fix_partition_shard_index_names +SELECT fix_partition_shard_index_names(NULL); + fix_partition_shard_index_names +--------------------------------------------------------------------- + +(1 row) + +SELECT worker_fix_partition_shard_index_names(NULL, NULL, NULL); + worker_fix_partition_shard_index_names +--------------------------------------------------------------------- + +(1 row) + +-- fix_partition_shard_index_names cannot be called for distributed +-- and not partitioned tables +CREATE TABLE not_partitioned(id int); +SELECT create_distributed_table('not_partitioned', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(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 +-- 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 +-- 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 +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 +--------------------------------------------------------------------- + dist_partitioned_table | short + p | p_another_col_partition_col_idx + partition_table_with_very_long_name | partition_table_with_very_long_na_another_col_partition_col_idx +(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 +-- 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; + tablename | indexname +--------------------------------------------------------------------- + dist_partitioned_table_910004 | short_910004 + dist_partitioned_table_910006 | short_910006 + p_910012 | p_another_col_partition_col_idx_910012 + p_910014 | p_another_col_partition_col_idx_910014 + 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 +(6 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +-- this should now work +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 +SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); + fix_partition_shard_index_names +--------------------------------------------------------------------- + +(1 row) + +SELECT fix_all_partition_shard_index_names(); + fix_all_partition_shard_index_names +--------------------------------------------------------------------- + dist_partitioned_table +(1 row) + +\c - - - :worker_1_port +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; +SET citus.shard_replication_factor TO 1; +SET citus.next_shard_id TO 910020; +-- 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); +ALTER TABLE dist_partitioned_table ATTACH PARTITION another_partition_table_with_very_long_name FOR VALUES FROM ('2020-01-01') TO ('2021-01-01'); +-- 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'); +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + another_partition_table_with_very_long_name | another_partition_table_with_very_another_col_partition_col_idx + dist_partitioned_table | short + p | p_another_col_partition_col_idx + partition_table_with_very_long_name | partition_table_with_very_long_na_another_col_partition_col_idx + yet_another_partition_table | really weird index name !! +(5 rows) + +\c - - - :worker_1_port +-- notice indexes of shards of another_partition_table_with_very_long_name already have shardid appended to the end +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + another_partition_table_with_very_long_name | another_partition_table_with_very_another_col_partition_col_idx + another_partition_table_with_very_long_name_910020 | another_partition_table_with_very_another_col_p_a02939b4_910020 + another_partition_table_with_very_long_name_910022 | another_partition_table_with_very_another_col_p_a02939b4_910022 + 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 + yet_another_partition_table | really weird index name !! + yet_another_partition_table_910024 | really weird index name !!_910024 + yet_another_partition_table_910026 | really weird index name !!_910026 +(15 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +-- this command would not do anything +SELECT fix_all_partition_shard_index_names(); + fix_all_partition_shard_index_names +--------------------------------------------------------------------- + dist_partitioned_table +(1 row) + +\c - - - :worker_1_port +-- names are the same as before +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + another_partition_table_with_very_long_name | another_partition_table_with_very_another_col_partition_col_idx + another_partition_table_with_very_long_name_910020 | another_partition_table_with_very_another_col_p_a02939b4_910020 + another_partition_table_with_very_long_name_910022 | another_partition_table_with_very_another_col_p_a02939b4_910022 + 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 + yet_another_partition_table | really weird index name !! + yet_another_partition_table_910024 | really weird index name !!_910024 + yet_another_partition_table_910026 | really weird index name !!_910026 +(15 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +SELECT stop_metadata_sync_to_node('localhost', :worker_1_port); +NOTICE: dropping metadata on the node (localhost,57637) + stop_metadata_sync_to_node +--------------------------------------------------------------------- + +(1 row) + +DROP INDEX short; +DROP TABLE yet_another_partition_table, another_partition_table_with_very_long_name; +-- this will create constraint1 index on parent +ALTER TABLE dist_partitioned_table ADD CONSTRAINT constraint1 UNIQUE (dist_col, partition_col); +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 +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 +--------------------------------------------------------------------- + dist_partitioned_table | constraint1 + dist_partitioned_table | dist_partitioned_table_dist_col_partition_col_idx + p | p_dist_col_partition_col_idx + p | p_dist_col_partition_col_key + partition_table_with_very_long_name | partition_table_with_very_long_name_dist_col_partition_col_idx + partition_table_with_very_long_name | partition_table_with_very_long_name_dist_col_partition_col_key +(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 +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_dist_col_partition_col_idx_910012 + p_910012 | p_dist_col_partition_col_key_910012 + p_910014 | p_dist_col_partition_col_idx_910014 + p_910014 | p_dist_col_partition_col_key_910014 + partition_table_with_very_long_name_910008 | partition_table_with_very_long_name_dist_col_pa_781a5400_910008 + partition_table_with_very_long_name_910008 | partition_table_with_very_long_name_dist_col_pa_ef25fb77_910008 + partition_table_with_very_long_name_910010 | partition_table_with_very_long_name_dist_col_pa_781a5400_910010 + partition_table_with_very_long_name_910010 | partition_table_with_very_long_name_dist_col_pa_ef25fb77_910010 +(12 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +ALTER TABLE dist_partitioned_table DROP CONSTRAINT constraint1 CASCADE; +NOTICE: drop cascades to constraint fk_table_id_fk_column_fkey on table fk_table +DROP INDEX dist_partitioned_table_dist_col_partition_col_idx; +-- 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); +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 +WHERE (i.indisvalid = false) AND i.indexrelid = c.oid AND c.relnamespace = n.oid AND n.nspname = 'fix_idx_names'; + indexname +--------------------------------------------------------------------- + short_parent +(1 row) + +-- try with index on only partition +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; + tablename | indexname +--------------------------------------------------------------------- + another_partition | another_partition_dist_col_idx + dist_partitioned_table | short_parent + p | short_child +(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 +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + another_partition_361176 | another_partition_dist_col_idx_361176 + another_partition_361178 | another_partition_dist_col_idx_361178 + 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; +DROP INDEX short_parent; +DROP INDEX short_child; +DROP TABLE another_partition; +-- expression indexes have the same problem with naming +CREATE INDEX expression_index ON dist_partitioned_table ((dist_col || ' ' || another_col)); +-- try with statistics on index +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; +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + dist_partitioned_table | expression_index + dist_partitioned_table | statistics_on_index + p | p_expr_expr1_idx + p | p_expr_idx + partition_table_with_very_long_name | partition_table_with_very_long_name_expr_expr1_idx + partition_table_with_very_long_name | partition_table_with_very_long_name_expr_idx +(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 +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_expr_expr1_idx_910012 + p_910012 | p_expr_idx_910012 + p_910014 | p_expr_expr1_idx_910014 + p_910014 | p_expr_idx_910014 + partition_table_with_very_long_name_910008 | partition_table_with_very_long_name_expr_expr1_idx_910008 + partition_table_with_very_long_name_910008 | partition_table_with_very_long_name_expr_idx_910008 + partition_table_with_very_long_name_910010 | partition_table_with_very_long_name_expr_expr1_idx_910010 + partition_table_with_very_long_name_910010 | partition_table_with_very_long_name_expr_idx_910010 +(12 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +-- try with a table with no partitions +ALTER TABLE dist_partitioned_table DETACH PARTITION p; +ALTER TABLE dist_partitioned_table DETACH PARTITION partition_table_with_very_long_name; +DROP TABLE p; +DROP TABLE partition_table_with_very_long_name; +-- still dist_partitioned_table has indexes +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + dist_partitioned_table | expression_index + dist_partitioned_table | statistics_on_index +(2 rows) + +-- this does nothing +SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); + fix_partition_shard_index_names +--------------------------------------------------------------------- + +(1 row) + +\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 +(4 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +DROP TABLE dist_partitioned_table; +-- add test with replication factor = 2 +SET citus.shard_replication_factor TO 2; +SET citus.next_shard_id TO 910050; +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 +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 +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 +--------------------------------------------------------------------- + dist_partitioned_table | index_rep_factor_2 + partition_table_with_very_long_name | partition_table_with_very_long_na_another_col_partition_col_idx +(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 +-- 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; + 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_p_dd884a3b_910054 + partition_table_with_very_long_name_910055 | partition_table_with_very_long_na_another_col_p_dd884a3b_910055 + partition_table_with_very_long_name_910056 | partition_table_with_very_long_na_another_col_p_dd884a3b_910056 + partition_table_with_very_long_name_910057 | partition_table_with_very_long_na_another_col_p_dd884a3b_910057 +(8 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +-- test with role that is not superuser +SET client_min_messages TO warning; +SET citus.enable_ddl_propagation TO off; +CREATE USER user1; +RESET client_min_messages; +RESET citus.enable_ddl_propagation; +SET ROLE user1; +SELECT fix_partition_shard_index_names('fix_idx_names.dist_partitioned_table'::regclass); +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 +-- 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; +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 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 INDEX short ON dist_partitioned_table USING btree (another_col, partition_col); +-- rename shouldn't work +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 +-- 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) +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + dist_partitioned_table_910030 | short_910030 + partition_table_with_very_long_name_910031 | partition_table_with_very_long_name_idx_910031 +(2 rows) + +\c - - - :master_port +SET search_path TO fix_idx_names, public; +DROP SCHEMA fix_idx_names CASCADE; +NOTICE: drop cascades to 5 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 +SELECT run_command_on_workers($$ DROP SCHEMA IF EXISTS fix_idx_names CASCADE $$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"DROP SCHEMA") + (localhost,57638,t,"DROP SCHEMA") +(2 rows) + diff --git a/src/test/regress/expected/multi_partitioning.out b/src/test/regress/expected/multi_partitioning.out index a59b9e975..ce11c3c47 100644 --- a/src/test/regress/expected/multi_partitioning.out +++ b/src/test/regress/expected/multi_partitioning.out @@ -3912,11 +3912,84 @@ CALL drop_old_time_partitions('non_partitioned_table', now()); ERROR: non_partitioned_table is not partitioned CONTEXT: PL/pgSQL function drop_old_time_partitions(regclass,timestamp with time zone) line XX at RAISE DROP TABLE non_partitioned_table; +-- https://github.com/citusdata/citus/issues/4962 +SET citus.shard_replication_factor TO 1; +CREATE TABLE part_table_with_very_long_name ( + dist_col integer, + long_named_integer_col integer, + long_named_part_col timestamp +) PARTITION BY RANGE (long_named_part_col); +CREATE TABLE part_table_with_long_long_long_long_name +PARTITION OF part_table_with_very_long_name +FOR VALUES FROM ('2010-01-01') TO ('2015-01-01'); +SELECT create_distributed_table('part_table_with_very_long_name', 'dist_col'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +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 +SELECT tablename, indexname FROM pg_indexes +WHERE schemaname = 'partitioning_schema' AND tablename ilike '%part_table_with_%' ORDER BY 1, 2; + 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) + +\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 +SELECT start_metadata_sync_to_node('localhost', :worker_1_port); + start_metadata_sync_to_node +--------------------------------------------------------------------- + +(1 row) + +\c - - - :worker_1_port +-- check that indexes are renamed +SELECT tablename, indexname FROM pg_indexes +WHERE schemaname = 'partitioning_schema' AND tablename ilike '%part_table_with_%' ORDER BY 1, 2; + tablename | indexname +--------------------------------------------------------------------- + part_table_with_long_long_long_long_name | part_table_with_long_long_lon_long_named_integer_col_long_n_idx + part_table_with_long_long_long_long_name_361172 | part_table_with_long_long_lon_long_named_intege_f9175544_361172 + part_table_with_long_long_long_long_name_361174 | part_table_with_long_long_lon_long_named_intege_f9175544_361174 + part_table_with_very_long_name | part_table_with_very_long_nam_long_named_integer_col_long_n_idx + 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 +(6 rows) + +\c - - - :master_port +SELECT stop_metadata_sync_to_node('localhost', :worker_1_port); +NOTICE: dropping metadata on the node (localhost,57637) + stop_metadata_sync_to_node +--------------------------------------------------------------------- + +(1 row) + DROP SCHEMA partitioning_schema CASCADE; -NOTICE: drop cascades to 3 other objects -DETAIL: drop cascades to table "schema-test" -drop cascades to table another_distributed_table -drop cascades to table distributed_parent_table +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to table partitioning_schema."schema-test" +drop cascades to table partitioning_schema.another_distributed_table +drop cascades to table partitioning_schema.distributed_parent_table +drop cascades to table partitioning_schema.part_table_with_very_long_name RESET search_path; DROP TABLE IF EXISTS partitioning_hash_test, diff --git a/src/test/regress/expected/upgrade_list_citus_objects.out b/src/test/regress/expected/upgrade_list_citus_objects.out index bb2efb550..c05821666 100644 --- a/src/test/regress/expected/upgrade_list_citus_objects.out +++ b/src/test/regress/expected/upgrade_list_citus_objects.out @@ -123,6 +123,8 @@ ORDER BY 1; function dump_global_wait_edges() function dump_local_wait_edges() function fetch_intermediate_results(text[],text,integer) + function fix_all_partition_shard_index_names() + function fix_partition_shard_index_names(regclass) function fix_pre_citus10_partitioned_table_constraint_names() function fix_pre_citus10_partitioned_table_constraint_names(regclass) function get_all_active_transactions() @@ -202,6 +204,7 @@ ORDER BY 1; function worker_drop_distributed_table(text) function worker_fetch_foreign_file(text,text,bigint,text[],integer[]) function worker_fetch_partition_file(bigint,integer,integer,integer,text,integer) + function worker_fix_partition_shard_index_names(regclass, text, text) function worker_fix_pre_citus10_partitioned_table_constraint_names(regclass,bigint,text) function worker_hash("any") function worker_hash_partition_table(bigint,integer,text,text,oid,anyarray) @@ -258,5 +261,5 @@ ORDER BY 1; view citus_worker_stat_activity view pg_dist_shard_placement view time_partitions -(242 rows) +(245 rows) diff --git a/src/test/regress/isolation_schedule b/src/test/regress/isolation_schedule index 037755a40..2ec4d649c 100644 --- a/src/test/regress/isolation_schedule +++ b/src/test/regress/isolation_schedule @@ -66,6 +66,7 @@ test: shared_connection_waits test: isolation_cancellation test: isolation_undistribute_table test: isolation_citus_update_table_statistics +test: isolation_fix_partition_shard_index_names # Rebalancer test: isolation_blocking_move_single_shard_commands diff --git a/src/test/regress/multi_1_schedule b/src/test/regress/multi_1_schedule index 266ee3e12..e49a22dd4 100644 --- a/src/test/regress/multi_1_schedule +++ b/src/test/regress/multi_1_schedule @@ -67,6 +67,7 @@ test: ensure_no_intermediate_data_leak # ---------- test: multi_partitioning_utils multi_partitioning partitioning_issue_3970 replicated_partitioned_table test: drop_partitioned_table +test: multi_fix_partition_shard_index_names test: partition_wise_join # ---------- 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 new file mode 100644 index 000000000..6c11978f9 --- /dev/null +++ b/src/test/regress/spec/isolation_fix_partition_shard_index_names.spec @@ -0,0 +1,48 @@ +setup +{ + CREATE TABLE dist_partitioned_table(a INT, created_at timestamptz) PARTITION BY RANGE (created_at); + SELECT create_distributed_table('dist_partitioned_table', 'a'); +} + +teardown +{ + DROP TABLE IF EXISTS dist_partitioned_table; +} + +session "s1" + +step "s1-begin" +{ + BEGIN; +} + +step "s1-drop-table" +{ + DROP TABLE dist_partitioned_table; +} + +step "s1-commit" +{ + COMMIT; +} + +session "s2" + +step "s2-begin" +{ + BEGIN; +} + +step "s2-fix-partition-shard-index-names" +{ + SET client_min_messages TO NOTICE; + SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); +} + +step "s2-commit" +{ + 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" 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 new file mode 100644 index 000000000..3202d20eb --- /dev/null +++ b/src/test/regress/sql/multi_fix_partition_shard_index_names.sql @@ -0,0 +1,321 @@ +---------------------------------------------------- +-- multi_fix_partition_shard_index_names +-- check the following two issues +-- https://github.com/citusdata/citus/issues/4962 +-- https://github.com/citusdata/citus/issues/5138 +---------------------------------------------------- +SET citus.next_shard_id TO 910000; +SET citus.shard_replication_factor TO 1; +CREATE SCHEMA fix_idx_names; +SET search_path TO fix_idx_names, public; + +-- NULL input should automatically return NULL since +-- fix_partition_shard_index_names is strict +-- same for worker_fix_partition_shard_index_names +SELECT fix_partition_shard_index_names(NULL); +SELECT worker_fix_partition_shard_index_names(NULL, NULL, NULL); + +-- fix_partition_shard_index_names cannot be called for distributed +-- and not partitioned tables +CREATE TABLE not_partitioned(id int); +SELECT create_distributed_table('not_partitioned', 'id'); +SELECT fix_partition_shard_index_names('not_partitioned'::regclass); + +-- 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); + +-- 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 +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 +-- 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 +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 +SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); +SELECT fix_all_partition_shard_index_names(); + +\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; +SET citus.shard_replication_factor TO 1; +SET citus.next_shard_id TO 910020; + +-- 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); +ALTER TABLE dist_partitioned_table ATTACH PARTITION another_partition_table_with_very_long_name FOR VALUES FROM ('2020-01-01') TO ('2021-01-01'); + +-- 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'); +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + +\c - - - :worker_1_port +-- notice indexes of shards of another_partition_table_with_very_long_name already have shardid appended 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 command would not do anything +SELECT fix_all_partition_shard_index_names(); + +\c - - - :worker_1_port +-- names are the same as before +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 stop_metadata_sync_to_node('localhost', :worker_1_port); + +DROP INDEX short; +DROP TABLE yet_another_partition_table, another_partition_table_with_very_long_name; +-- this will create constraint1 index on parent +ALTER TABLE dist_partitioned_table ADD CONSTRAINT constraint1 UNIQUE (dist_col, partition_col); +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 +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 +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; + +ALTER TABLE dist_partitioned_table DROP CONSTRAINT constraint1 CASCADE; +DROP INDEX dist_partitioned_table_dist_col_partition_col_idx; + +-- 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); +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 +WHERE (i.indisvalid = false) AND i.indexrelid = c.oid AND c.relnamespace = n.oid AND n.nspname = 'fix_idx_names'; + +-- try with index on only partition +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 +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; + +DROP INDEX short_parent; +DROP INDEX short_child; +DROP TABLE another_partition; + +-- expression indexes have the same problem with naming +CREATE INDEX expression_index ON dist_partitioned_table ((dist_col || ' ' || another_col)); +-- try with statistics on index +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; + +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 +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; + +-- try with a table with no partitions +ALTER TABLE dist_partitioned_table DETACH PARTITION p; +ALTER TABLE dist_partitioned_table DETACH PARTITION partition_table_with_very_long_name; +DROP TABLE p; +DROP TABLE partition_table_with_very_long_name; + +-- still dist_partitioned_table has indexes +SELECT tablename, indexname FROM pg_indexes WHERE schemaname = 'fix_idx_names' ORDER BY 1, 2; + +-- this does nothing +SELECT fix_partition_shard_index_names('dist_partitioned_table'::regclass); + +\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; +DROP TABLE dist_partitioned_table; + +-- add test with replication factor = 2 +SET citus.shard_replication_factor TO 2; +SET citus.next_shard_id TO 910050; + +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 +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 +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 +-- 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; + +-- test with role that is not superuser +SET client_min_messages TO warning; +SET citus.enable_ddl_propagation TO off; +CREATE USER user1; +RESET client_min_messages; +RESET citus.enable_ddl_propagation; + +SET ROLE user1; +SELECT fix_partition_shard_index_names('fix_idx_names.dist_partitioned_table'::regclass); + +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 + +-- 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; + +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 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 INDEX short ON dist_partitioned_table USING btree (another_col, partition_col); + +-- rename shouldn't work +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 +-- 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) +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; + +DROP SCHEMA fix_idx_names CASCADE; +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 39fc301e8..87a958b2d 100644 --- a/src/test/regress/sql/multi_partitioning.sql +++ b/src/test/regress/sql/multi_partitioning.sql @@ -1886,6 +1886,46 @@ SELECT create_time_partitions('non_partitioned_table', INTERVAL '1 month', now() CALL drop_old_time_partitions('non_partitioned_table', now()); DROP TABLE non_partitioned_table; +-- https://github.com/citusdata/citus/issues/4962 +SET citus.shard_replication_factor TO 1; +CREATE TABLE part_table_with_very_long_name ( + dist_col integer, + long_named_integer_col integer, + long_named_part_col timestamp +) PARTITION BY RANGE (long_named_part_col); + +CREATE TABLE part_table_with_long_long_long_long_name +PARTITION OF part_table_with_very_long_name +FOR VALUES FROM ('2010-01-01') TO ('2015-01-01'); + +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 +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 +SELECT start_metadata_sync_to_node('localhost', :worker_1_port); + +\c - - - :worker_1_port +-- check that indexes are renamed +SELECT tablename, indexname FROM pg_indexes +WHERE schemaname = 'partitioning_schema' AND tablename ilike '%part_table_with_%' ORDER BY 1, 2; + +\c - - - :master_port +SELECT stop_metadata_sync_to_node('localhost', :worker_1_port); + DROP SCHEMA partitioning_schema CASCADE; RESET search_path; DROP TABLE IF EXISTS