Remove the cache optimization of cdbRelSize, and rename it to avoid

misunderstanding, for following reasons:
(1) the performance improvement is little, it only works for some code
paths of ALTER TABLE ALTER COLUMN TYPE and CREATE UNIQUE/PRIMARY INDEX
(2) the feature is incomplete and buggy, especially the cache invalidation part

src/backend/cdb/cdbcat.c

@@ -532,7 +532,7 @@ checkPolicyForUniqueIndex(Relation rel, AttrNumber *indattr, int nidxatts,
 	 */
 	if (!bms_is_subset(polbm, indbm))
 	{
-		if (cdbRelSize(rel) != 0 || has_pkey || has_ukey || has_exprs)
+		if (cdbRelMaxSegSize(rel) != 0 || has_pkey || has_ukey || has_exprs)
 		{
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),

src/backend/cdb/cdbrelsize.c

 /*-------------------------------------------------------------------------
  *
- * cdbrelsize.h
- *
- * Get the max size of the relation across the segDBs
+ * cdbrelsize.c
  *
  * Copyright (c) 2006-2008, Greenplum inc
  *
- *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
@@ -28,49 +25,21 @@
 
 #include "cdb/cdbrelsize.h"
 
-#define relsize_cache_size 100
-
-struct relsize_cache_entry 
-{
-	Oid	relOid;
-	int64 size;
-};
-
-static struct relsize_cache_entry relsize_cache[relsize_cache_size] = { {0,0} };
-
-static int last_cache_entry = -1;		/* -1 for cache not initialized yet */
-
-void clear_relsize_cache(void)
+/*
+ * Get the max size of the relation across segments
+ */
+int64
+cdbRelMaxSegSize(Relation rel)
 {
+	int64 size = 0;
 	int i;
-	for (i=0; i < relsize_cache_size; i++)
-	{
-		relsize_cache[i].relOid = InvalidOid;
-		relsize_cache[i].size = 0;
-	}
-	last_cache_entry = -1;
-}
-
-int64 cdbRelSize(Relation rel)
-{
-	int64	size = 0;
-	int		i;
-	int 	resultCount = 0;
+	int resultCount = 0;
 	struct pg_result **results = NULL;
 	StringInfoData errbuf;
 	StringInfoData buffer;
 
-	char	*schemaName;
-	char	*relName;	
-
-	if (last_cache_entry  >= 0)
-	{
-		for (i=0; i < relsize_cache_size; i++)
-		{
-			if (relsize_cache[i].relOid == RelationGetRelid(rel))
-				return relsize_cache[i].size;
-		}
-	}
+	char *schemaName;
+	char *relName;
 
 	/*
 	 * Let's ask the QEs for the size of the relation
@@ -84,14 +53,14 @@ int64 cdbRelSize(Relation rel)
 			 RelationGetNamespace(rel));
 	relName = RelationGetRelationName(rel);
 
-	/* 
+	/*
 	 * Safer to pass names than oids, just in case they get out of sync between QD and QE,
 	 * which might happen with a toast table or index, I think (but maybe not)
 	 */
 	appendStringInfo(&buffer, "select pg_relation_size('%s.%s')",
 					 quote_identifier(schemaName), quote_identifier(relName));
 
-	/* 
+	/*
 	 * In the future, it would be better to send the command to only one QE for the optimizer's needs,
 	 * but for ALTER TABLE, we need to be sure if the table has any rows at all.
 	 */
@@ -99,8 +68,9 @@ int64 cdbRelSize(Relation rel)
 
 	if (errbuf.len > 0)
 	{
-		ereport(WARNING, (errmsg("cdbRelSize error (gathered %d results from cmd '%s')", resultCount, buffer.data),
-						  errdetail("%s", errbuf.data)));
+		ereport(WARNING,
+				(errmsg("cdbRelMaxSegSize error (gathered %d results from cmd '%s')", resultCount, buffer.data),
+				 errdetail("%s", errbuf.data)));
 		pfree(errbuf.data);
 		pfree(buffer.data);
 		
@@ -108,48 +78,30 @@ int64 cdbRelSize(Relation rel)
 	}
 	else
 	{
-										
 		for (i = 0; i < resultCount; i++)
 		{
 			if (PQresultStatus(results[i]) != PGRES_TUPLES_OK)
 			{
-				elog(ERROR,"cdbRelSize: resultStatus not tuples_Ok: %s   %s",PQresStatus(PQresultStatus(results[i])),PQresultErrorMessage(results[i]));
+				elog(ERROR,"cdbRelMaxSegSize: resultStatus not tuples_Ok: %s %s",
+					 PQresStatus(PQresultStatus(results[i])),PQresultErrorMessage(results[i]));
 			}
 			else
 			{
-				/*
-				 * Due to funkyness in the current dispatch agent code, instead of 1 result 
-				 * per QE with 1 row each, we can get back 1 result per dispatch agent, with
-				 * one row per QE controlled by that agent.
-				 */
-				int j;
-				for (j = 0; j < PQntuples(results[i]); j++)
-				{
-					int64 tempsize = 0;
-					(void) scanint8(PQgetvalue(results[i], j, 0), false, &tempsize);
-					if (tempsize > size)
-					 	size = tempsize;
-				}
+				Assert(PQntuples(results[i]) == 1);
+				int64 tempsize = 0;
+				(void) scanint8(PQgetvalue(results[i], 0, 0), false, &tempsize);
+				if (tempsize > size)
+					size = tempsize;
 			}
 		}
-	
+
 		pfree(errbuf.data);
 		pfree(buffer.data);
 
 		for (i = 0; i < resultCount; i++)
 			PQclear(results[i]);
-	
-		free(results);
-	}
 
-	if (size >= 0)	/* Cache the size even if it is zero, as table might be empty */
-	{
-		if (last_cache_entry < 0)
-			last_cache_entry = 0;
-
-		relsize_cache[last_cache_entry].relOid = RelationGetRelid(rel);
-		relsize_cache[last_cache_entry].size = size;
-		last_cache_entry = (last_cache_entry+1) % relsize_cache_size;
+		free(results);
 	}
 
 	return size;

src/backend/cdb/dispatcher/cdbdisp_query.c

@@ -16,7 +16,6 @@
 #include "cdb/cdbutil.h"
 #include "cdb/cdbvars.h"
 #include "cdb/cdbmutate.h"
-#include "cdb/cdbrelsize.h"
 #include "cdb/cdbsrlz.h"
 #include "tcop/tcopprot.h"
 #include "utils/datum.h"
@@ -180,7 +179,7 @@ cdbdisp_dispatchPlan(struct QueryDesc *queryDesc,
 	int rootIdx;
 	int oldLocalSlice;
 	PlannedStmt *stmt;
-	bool is_SRI;
+	bool is_SRI = false;
 
 	DispatchCommandQueryParms queryParms;
 	CdbComponentDatabaseInfo *qdinfo;
@@ -222,8 +221,6 @@ cdbdisp_dispatchPlan(struct QueryDesc *queryDesc,
 	 * nextval() and currval() now, so that we get the QD's values, and a
 	 * consistent value for everyone
 	 */
-	is_SRI = false;
-
 	if (queryDesc->operation == CMD_INSERT)
 	{
 		Assert(stmt->commandType == CMD_INSERT);
@@ -232,13 +229,9 @@ cdbdisp_dispatchPlan(struct QueryDesc *queryDesc,
 		 * We might look for constant input relation (instead of SRI), but I'm afraid
 		 * that wouldn't scale.
 		 */
-		is_SRI = IsA(stmt->planTree, Result)
-			&& stmt->planTree->lefttree == NULL;
+		is_SRI = IsA(stmt->planTree, Result) && stmt->planTree->lefttree == NULL;
 	}
 
-	if (!is_SRI)
-		clear_relsize_cache();
-
 	if (queryDesc->operation == CMD_INSERT ||
 		queryDesc->operation == CMD_SELECT ||
 		queryDesc->operation == CMD_UPDATE ||

src/backend/cdb/dispatcher/test/cdbdisp_query_test.c

@@ -17,7 +17,6 @@ _init_cdbdisp_dispatchPlan(QueryDesc *queryDesc)
 	queryDesc->operation = CMD_NOTHING;
 	queryDesc->plannedstmt = (PlannedStmt *)palloc0(sizeof(PlannedStmt));
 
-	will_be_called(clear_relsize_cache);
 	expect_any(RootSliceIndex, estate);
 	will_return(RootSliceIndex,0);
 }

src/backend/commands/copy.c

@@ -61,7 +61,6 @@
 #include "cdb/cdbhash.h"
 #include "cdb/cdbdispatchresult.h"
 #include "cdb/cdbsreh.h"
-#include "cdb/cdbrelsize.h"
 #include "cdb/cdbutil.h"
 #include "cdb/cdbvarblock.h"
 #include "cdb/cdbbufferedappend.h"

src/backend/commands/tablecmds.c

@@ -9237,7 +9237,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 								{
 									int ia = 0;
 
-									if (cdbRelSize(rel) != 0)
+									if (cdbRelMaxSegSize(rel) != 0)
 									{
 										for (ia = 0; ia < policy->nattrs; ia++)
 										{
@@ -9287,7 +9287,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 						{
 							int ia = 0;
 
-							if (cdbRelSize(rel) != 0)
+							if (cdbRelMaxSegSize(rel) != 0)
 							{
 								for (ia = 0; ia < policy->nattrs; ia++)
 								{
@@ -9425,7 +9425,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 									"a partitioning key")));
 			}
 
-			if (cdbRelSize(rel) != 0)
+			if (cdbRelMaxSegSize(rel) != 0)
 			{
 				for (ia = 0; ia < policy->nattrs; ia++)
 				{

src/backend/commands/vacuum.c

@@ -49,7 +49,6 @@
 #include "cdb/cdbpartition.h"
 #include "cdb/cdbvars.h"
 #include "cdb/cdbsrlz.h"
-#include "cdb/cdbrelsize.h"
 #include "cdb/cdbdispatchresult.h"      /* CdbDispatchResults */
 #include "cdb/cdbfilerepprimary.h"
 #include "cdb/cdbpersistentfilesysobj.h"
@@ -382,9 +381,6 @@ vacuum(VacuumStmt *vacstmt, List *relids,
 	if (Gp_role == GP_ROLE_DISPATCH)
 		elevel = DEBUG2; /* vacuum messages aren't interesting from the QD */
 
-	if (Gp_role == GP_ROLE_DISPATCH)
-		clear_relsize_cache();
-
 	/*
 	 * We cannot run VACUUM inside a user transaction block; if we were inside
 	 * a transaction, then our commit- and start-transaction-command calls

src/include/cdb/cdbrelsize.h

@@ -14,9 +14,7 @@
 
 #include "utils/relcache.h"
 
-extern void clear_relsize_cache(void);
-
-extern int64 cdbRelSize(Relation rel);
+extern int64 cdbRelMaxSegSize(Relation rel);
 
 
 #endif /*CDBRELSIZE_H_*/