From cf775c47735b0ec19de3d0a4bad33c1222d9f437 Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Thu, 30 Mar 2017 01:13:39 -0600 Subject: [PATCH] Improve CONCURRENTLY-related error messages Thought this looked slightly nicer than the default behavior. Changed preventTransaction to concurrent to be clearer that this code path presently affects CONCURRENTLY code only. --- .../distributed/executor/multi_utility.c | 57 ++++++++++++------- src/include/distributed/multi_utility.h | 2 +- .../expected/multi_index_statements.out | 10 ++-- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index eece32ed2..485ff2da7 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -684,7 +684,7 @@ PlanIndexStmt(IndexStmt *createIndexStatement, const char *createIndexCommand) { DDLJob *ddlJob = palloc0(sizeof(DDLJob)); ddlJob->targetRelationId = relationId; - ddlJob->preventTransaction = createIndexStatement->concurrent; + ddlJob->concurrent = createIndexStatement->concurrent; ddlJob->commandString = createIndexCommand; ddlJob->taskList = IndexTaskList(relationId, createIndexStatement); @@ -778,7 +778,7 @@ PlanDropIndexStmt(DropStmt *dropIndexStatement, const char *dropIndexCommand) ErrorIfUnsupportedDropIndexStmt(dropIndexStatement); ddlJob->targetRelationId = distributedRelationId; - ddlJob->preventTransaction = dropIndexStatement->concurrent; + ddlJob->concurrent = dropIndexStatement->concurrent; ddlJob->commandString = dropIndexCommand; ddlJob->taskList = DropIndexTaskList(distributedRelationId, distributedIndexId, dropIndexStatement); @@ -873,7 +873,7 @@ PlanAlterTableStmt(AlterTableStmt *alterTableStatement, const char *alterTableCo ddlJob = palloc0(sizeof(DDLJob)); ddlJob->targetRelationId = leftRelationId; - ddlJob->preventTransaction = false; + ddlJob->concurrent = false; ddlJob->commandString = alterTableCommand; if (rightRelationId) @@ -1987,24 +1987,7 @@ ExecuteDistributedDDLJob(DDLJob *ddlJob) EnsureCoordinator(); - if (ddlJob->preventTransaction) - { - /* save old commit protocol to restore at xact end */ - Assert(SavedMultiShardCommitProtocol == COMMIT_PROTOCOL_BARE); - SavedMultiShardCommitProtocol = MultiShardCommitProtocol; - MultiShardCommitProtocol = COMMIT_PROTOCOL_BARE; - - if (shouldSyncMetadata) - { - List *commandList = list_make2(DISABLE_DDL_PROPAGATION, - (char *) ddlJob->commandString); - - SendBareCommandListToWorkers(WORKERS_WITH_METADATA, commandList); - } - - ExecuteSequentialTasksWithoutResults(ddlJob->taskList); - } - else + if (!ddlJob->concurrent) { ShowNoticeIfNotUsing2PC(); @@ -2016,6 +1999,36 @@ ExecuteDistributedDDLJob(DDLJob *ddlJob) ExecuteModifyTasksWithoutResults(ddlJob->taskList); } + else + { + /* save old commit protocol to restore at xact end */ + Assert(SavedMultiShardCommitProtocol == COMMIT_PROTOCOL_BARE); + SavedMultiShardCommitProtocol = MultiShardCommitProtocol; + MultiShardCommitProtocol = COMMIT_PROTOCOL_BARE; + + PG_TRY(); + { + if (shouldSyncMetadata) + { + List *commandList = list_make2(DISABLE_DDL_PROPAGATION, + (char *) ddlJob->commandString); + + SendBareCommandListToWorkers(WORKERS_WITH_METADATA, commandList); + } + + ExecuteSequentialTasksWithoutResults(ddlJob->taskList); + } + PG_CATCH(); + { + ereport(ERROR, + (errmsg("CONCURRENTLY-enabled index command failed"), + errdetail("CONCURRENTLY-enabled index commands can fail partially, " + "leaving behind an INVALID index."), + errhint("Use DROP INDEX IF EXISTS to remove the invalid index, then " + "retry the original command."))); + } + PG_END_TRY(); + } } @@ -2701,7 +2714,7 @@ PlanGrantStmt(GrantStmt *grantStmt) ddlJob = palloc0(sizeof(DDLJob)); ddlJob->targetRelationId = relOid; - ddlJob->preventTransaction = false; + ddlJob->concurrent = false; ddlJob->commandString = pstrdup(ddlString.data); ddlJob->taskList = DDLTaskList(relOid, ddlString.data); diff --git a/src/include/distributed/multi_utility.h b/src/include/distributed/multi_utility.h index e6a4dfacb..8afad5d54 100644 --- a/src/include/distributed/multi_utility.h +++ b/src/include/distributed/multi_utility.h @@ -23,7 +23,7 @@ extern bool EnableDDLPropagation; typedef struct DDLJob { Oid targetRelationId; /* oid of the target distributed relation */ - bool preventTransaction; /* prevent use of worker transactions? */ + bool concurrent; /* related to a CONCURRENTLY index command? */ const char *commandString; /* initial (coordinator) DDL command string */ List *taskList; /* worker DDL tasks to execute */ } DDLJob; diff --git a/src/test/regress/expected/multi_index_statements.out b/src/test/regress/expected/multi_index_statements.out index 8e2be889e..3cce9ad30 100644 --- a/src/test/regress/expected/multi_index_statements.out +++ b/src/test/regress/expected/multi_index_statements.out @@ -250,8 +250,9 @@ CREATE INDEX CONCURRENTLY ith_b_idx_102089 ON index_test_hash_102089(b); \c - - - :master_port -- should fail because worker index already exists CREATE INDEX CONCURRENTLY ith_b_idx ON index_test_hash(b); -ERROR: relation "ith_b_idx_102089" already exists -CONTEXT: while executing command on localhost:57637 +ERROR: CONCURRENTLY-enabled index command failed +DETAIL: CONCURRENTLY-enabled index commands can fail partially, leaving behind an INVALID index. +HINT: Use DROP INDEX IF EXISTS to remove the invalid index, then retry the original command. -- the failure results in an INVALID index SELECT indisvalid AS "Index Valid?" FROM pg_index WHERE indexrelid='ith_b_idx'::regclass; Index Valid? @@ -273,8 +274,9 @@ SELECT indisvalid AS "Index Valid?" FROM pg_index WHERE indexrelid='ith_b_idx':: DROP INDEX CONCURRENTLY ith_b_idx_102089; \c - - - :master_port DROP INDEX CONCURRENTLY ith_b_idx; -ERROR: index "ith_b_idx_102089" does not exist -CONTEXT: while executing command on localhost:57637 +ERROR: CONCURRENTLY-enabled index command failed +DETAIL: CONCURRENTLY-enabled index commands can fail partially, leaving behind an INVALID index. +HINT: Use DROP INDEX IF EXISTS to remove the invalid index, then retry the original command. -- the failure results in an INVALID index SELECT indisvalid AS "Index Valid?" FROM pg_index WHERE indexrelid='ith_b_idx'::regclass; Index Valid?