Don't try to fetch table details using the old name after ExecRenameStmt

This used to work before PG 10, but some changes must have caused
non-deterministic behaviour. It anyways seems unsafe to lookup the catalogs
using the old name once ExecRenameStmt has finished. The lookup may or may not
see the old tuple, depending on whether CommandCounterIncrement has happened in
between. We now fetch the requried details before calling ExecRenameStmt and
use that info for subsequent processing.

This fixes some wierd issues in 'alter_table' test case where we were failing
to send ALTER TABLE RENAME TO command to remote nodes and causing inconsistent
catalog entries between the coordinator and the remote nodes.

1 files changed, 38 insertions, 36 deletions

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index e6687ab3b2..1f00a4c651 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1266,56 +1266,58 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 		case T_RenameStmt:
 			{
 				RenameStmt *stmt = (RenameStmt *) parsetree;
-
-				if (EventTriggerSupportsObjectType(stmt->renameType))
-					ProcessUtilitySlow(pstate, pstmt, queryString,
-									   context, params, queryEnv,
-									   dest,
-									   sentToRemote,
-									   completionTag);
-				else
-					ExecRenameStmt(stmt);
-			}
-#ifdef PGXC
-			if (IS_PGXC_LOCAL_COORDINATOR)
-			{
-				RenameStmt *stmt = (RenameStmt *) parsetree;
 				RemoteQueryExecType exec_type;
 				bool is_temp = false;
 
-				/* Try to use the object relation if possible */
-				if (stmt->relation)
+				if (IS_PGXC_LOCAL_COORDINATOR)
 				{
 					/*
-					 * When a relation is defined, it is possible that this object does
-					 * not exist but an IF EXISTS clause might be used. So we do not do
-					 * any error check here but block the access to remote nodes to
-					 * this object as it does not exisy
+					 * Get the necessary details about the relation before we
+					 * run ExecRenameStmt locally. Otherwise we may not be able
+					 * to look-up using the old relation name.
 					 */
-					Oid relid = RangeVarGetRelid(stmt->relation, NoLock, true);
+					if (stmt->relation)
+					{
+						/*
+						 * If the table does not exist, don't send the query to
+						 * the remote nodes. The local node will eventually
+						 * report an error, which is then sent back to the
+						 * client.
+						 */
+						Oid relid = RangeVarGetRelid(stmt->relation, NoLock, true);
 
-					if (OidIsValid(relid))
+						if (OidIsValid(relid))
+							exec_type = ExecUtilityFindNodes(stmt->renameType,
+									relid,
+									&is_temp);
+						else
+							exec_type = EXEC_ON_NONE;
+					}
+					else
 						exec_type = ExecUtilityFindNodes(stmt->renameType,
-								relid,
+								InvalidOid,
 								&is_temp);
-					else
-						exec_type = EXEC_ON_NONE;
 				}
+
+				if (EventTriggerSupportsObjectType(stmt->renameType))
+					ProcessUtilitySlow(pstate, pstmt, queryString,
+									   context, params, queryEnv,
+									   dest,
+									   sentToRemote,
+									   completionTag);
 				else
+					ExecRenameStmt(stmt);
+
+				if (IS_PGXC_LOCAL_COORDINATOR)
 				{
-					exec_type = ExecUtilityFindNodes(stmt->renameType,
-							InvalidOid,
-							&is_temp);
+					ExecUtilityStmtOnNodes(queryString,
+							NULL,
+							sentToRemote,
+							false,
+							exec_type,
+							is_temp);
 				}
-
-				ExecUtilityStmtOnNodes(queryString,
-						NULL,
-						sentToRemote,
-						false,
-						exec_type,
-						is_temp);
 			}
-#endif
 			break;
 
 		case T_AlterObjectDependsStmt: