Merge pull request #5465 from citusdata/marcocitus/remove-cstore_fdw

src/backend/distributed/metadata/metadata_sync.c

@@ -2282,8 +2282,7 @@ EnsureShardMetadataIsSane(Oid relationId, int64 shardId, char storageType,
 	}
 
 	if (!(storageType == SHARD_STORAGE_TABLE ||
-		  storageType == SHARD_STORAGE_FOREIGN ||
-		  storageType == SHARD_STORAGE_COLUMNAR))
+		  storageType == SHARD_STORAGE_FOREIGN))
 	{
 		ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 						errmsg("Invalid shard storage type: %c", storageType)));

src/backend/distributed/metadata/metadata_utility.c

@@ -276,12 +276,6 @@ citus_total_relation_size(PG_FUNCTION_ARGS)
 	bool failOnError = PG_GETARG_BOOL(1);
 
 	SizeQueryType sizeQueryType = TOTAL_RELATION_SIZE;
-
-	if (CStoreTable(relationId))
-	{
-		sizeQueryType = CSTORE_TABLE_SIZE;
-	}
-
 	uint64 tableSize = 0;
 
 	if (!DistributedTableSize(relationId, sizeQueryType, failOnError, &tableSize))
@@ -306,12 +300,6 @@ citus_table_size(PG_FUNCTION_ARGS)
 	Oid relationId = PG_GETARG_OID(0);
 	bool failOnError = true;
 	SizeQueryType sizeQueryType = TABLE_SIZE;
-
-	if (CStoreTable(relationId))
-	{
-		sizeQueryType = CSTORE_TABLE_SIZE;
-	}
-
 	uint64 tableSize = 0;
 
 	if (!DistributedTableSize(relationId, sizeQueryType, failOnError, &tableSize))
@@ -336,12 +324,6 @@ citus_relation_size(PG_FUNCTION_ARGS)
 	Oid relationId = PG_GETARG_OID(0);
 	bool failOnError = true;
 	SizeQueryType sizeQueryType = RELATION_SIZE;
-
-	if (CStoreTable(relationId))
-	{
-		sizeQueryType = CSTORE_TABLE_SIZE;
-	}
-
 	uint64 relationSize = 0;
 
 	if (!DistributedTableSize(relationId, sizeQueryType, failOnError, &relationSize))
@@ -861,11 +843,6 @@ GetSizeQueryBySizeQueryType(SizeQueryType sizeQueryType)
 			return PG_TOTAL_RELATION_SIZE_FUNCTION;
 		}
 
-		case CSTORE_TABLE_SIZE:
-		{
-			return CSTORE_TABLE_SIZE_FUNCTION;
-		}
-
 		case TABLE_SIZE:
 		{
 			return PG_TABLE_SIZE_FUNCTION;

src/backend/distributed/operations/delete_protocol.c

@@ -462,8 +462,7 @@ CreateDropShardPlacementCommand(const char *schemaName, const char *shardRelatio
 		appendStringInfo(workerDropQuery, DROP_REGULAR_TABLE_COMMAND,
 						 quotedShardName);
 	}
-	else if (storageType == SHARD_STORAGE_COLUMNAR ||
-			 storageType == SHARD_STORAGE_FOREIGN)
+	else if (storageType == SHARD_STORAGE_FOREIGN)
 	{
 		appendStringInfo(workerDropQuery, DROP_FOREIGN_TABLE_COMMAND,
 						 quotedShardName);

src/backend/distributed/operations/node_protocol.c

@@ -104,32 +104,6 @@ master_get_table_metadata(PG_FUNCTION_ARGS)
 }
 
 
-/*
- * CStoreTable returns true if the given relationId belongs to a foreign cstore
- * table, otherwise it returns false.
- */
-bool
-CStoreTable(Oid relationId)
-{
-	bool cstoreTable = false;
-
-	char relationKind = get_rel_relkind(relationId);
-	if (relationKind == RELKIND_FOREIGN_TABLE)
-	{
-		ForeignTable *foreignTable = GetForeignTable(relationId);
-		ForeignServer *server = GetForeignServer(foreignTable->serverid);
-		ForeignDataWrapper *foreignDataWrapper = GetForeignDataWrapper(server->fdwid);
-
-		if (strncmp(foreignDataWrapper->fdwname, CSTORE_FDW_NAME, NAMEDATALEN) == 0)
-		{
-			cstoreTable = true;
-		}
-	}
-
-	return cstoreTable;
-}
-
-
 /*
  * master_get_table_ddl_events takes in a relation name, and returns the set of
  * DDL commands needed to reconstruct the relation. The returned DDL commands
@@ -845,15 +819,7 @@ ShardStorageType(Oid relationId)
 	}
 	else if (relationType == RELKIND_FOREIGN_TABLE)
 	{
-		bool cstoreTable = CStoreTable(relationId);
-		if (cstoreTable)
-		{
-			shardStorageType = SHARD_STORAGE_COLUMNAR;
-		}
-		else
-		{
-			shardStorageType = SHARD_STORAGE_FOREIGN;
-		}
+		shardStorageType = SHARD_STORAGE_FOREIGN;
 	}
 	else
 	{

src/backend/distributed/operations/stage_protocol.c

@@ -129,24 +129,10 @@ master_create_empty_shard(PG_FUNCTION_ARGS)
 	/* don't allow concurrent node list changes that require an exclusive lock */
 	LockRelationOid(DistNodeRelationId(), RowShareLock);
 
-	/*
-	 * We check whether the table is a foreign table or not. If it is, we set
-	 * storage type as foreign also. Only exception is if foreign table is a
-	 * foreign cstore table, in this case we set storage type as columnar.
-	 *
-	 * i.e. While setting storage type, columnar has priority over foreign.
-	 */
+	/* set the storage type of foreign tables to 'f' */
 	if (relationKind == RELKIND_FOREIGN_TABLE)
 	{
-		bool cstoreTable = CStoreTable(relationId);
-		if (cstoreTable)
-		{
-			storageType = SHARD_STORAGE_COLUMNAR;
-		}
-		else
-		{
-			storageType = SHARD_STORAGE_FOREIGN;
-		}
+		storageType = SHARD_STORAGE_FOREIGN;
 	}
 
 	if (IsCitusTableType(relationId, HASH_DISTRIBUTED))
@@ -969,16 +955,7 @@ WorkerShardStats(ShardPlacement *placement, Oid relationId, const char *shardNam
 	*shardMaxValue = NULL;
 
 	char *quotedShardName = quote_literal_cstr(shardName);
-
-	bool cstoreTable = CStoreTable(relationId);
-	if (cstoreTable)
-	{
-		appendStringInfo(tableSizeQuery, SHARD_CSTORE_TABLE_SIZE_QUERY, quotedShardName);
-	}
-	else
-	{
-		appendStringInfo(tableSizeQuery, SHARD_TABLE_SIZE_QUERY, quotedShardName);
-	}
+	appendStringInfo(tableSizeQuery, SHARD_TABLE_SIZE_QUERY, quotedShardName);
 
 	int executeCommand = ExecuteOptionalRemoteCommand(connection, tableSizeQuery->data,
 													  &queryResult);

src/backend/distributed/sql/citus--10.2-4--11.0-1.sql

@@ -13,3 +13,13 @@ UPDATE pg_catalog.pg_dist_partition SET autoconverted = TRUE WHERE partmethod =
 
 REVOKE ALL ON FUNCTION start_metadata_sync_to_node(text, integer) FROM PUBLIC;
 REVOKE ALL ON FUNCTION stop_metadata_sync_to_node(text, integer,bool) FROM PUBLIC;
+
+DO LANGUAGE plpgsql
+$$
+BEGIN
+    IF EXISTS (SELECT 1 FROM pg_dist_shard where shardstorage = 'c') THEN
+	    RAISE EXCEPTION 'cstore_fdw tables are deprecated as of Citus 11.0'
+        USING HINT = 'Install Citus 10.2 and convert your cstore_fdw tables to the columnar access method before upgrading further';
+	END IF;
+END;
+$$;

src/include/distributed/coordinator_protocol.h

@@ -51,9 +51,6 @@
 #define TRANSFER_MODE_FORCE_LOGICAL 'l'
 #define TRANSFER_MODE_BLOCK_WRITES 'b'
 
-/* Name of columnar foreign data wrapper */
-#define CSTORE_FDW_NAME "cstore_fdw"
-
 #define SHARDID_SEQUENCE_NAME "pg_dist_shardid_seq"
 #define PLACEMENTID_SEQUENCE_NAME "pg_dist_placement_placementid_seq"
 
@@ -207,7 +204,6 @@ extern int NextPlacementId;
 extern bool IsCoordinator(void);
 
 /* Function declarations local to the distributed module */
-extern bool CStoreTable(Oid relationId);
 extern uint64 GetNextShardId(void);
 extern uint64 GetNextPlacementId(void);
 extern Oid ResolveRelationId(text *relationName, bool missingOk);

src/include/distributed/metadata_utility.h

@@ -34,7 +34,6 @@
 #define PG_TABLE_SIZE_FUNCTION "pg_table_size(%s)"
 #define PG_RELATION_SIZE_FUNCTION "pg_relation_size(%s)"
 #define PG_TOTAL_RELATION_SIZE_FUNCTION "pg_total_relation_size(%s)"
-#define CSTORE_TABLE_SIZE_FUNCTION "cstore_table_size(%s)"
 #define WORKER_PARTITIONED_TABLE_SIZE_FUNCTION "worker_partitioned_table_size(%s)"
 #define WORKER_PARTITIONED_RELATION_SIZE_FUNCTION "worker_partitioned_relation_size(%s)"
 #define WORKER_PARTITIONED_RELATION_TOTAL_SIZE_FUNCTION \
@@ -191,8 +190,7 @@ typedef enum SizeQueryType
 {
 	RELATION_SIZE, /* pg_relation_size() */
 	TOTAL_RELATION_SIZE, /* pg_total_relation_size() */
-	TABLE_SIZE, /* pg_table_size() */
-	CSTORE_TABLE_SIZE /* cstore_table_size() */
+	TABLE_SIZE /* pg_table_size() */
 } SizeQueryType;
 
 

src/include/distributed/pg_dist_shard.h

@@ -57,7 +57,6 @@ typedef FormData_pg_dist_shard *Form_pg_dist_shard;
  */
 #define SHARD_STORAGE_FOREIGN 'f'
 #define SHARD_STORAGE_TABLE 't'
-#define SHARD_STORAGE_COLUMNAR 'c'
 #define SHARD_STORAGE_VIRTUAL 'v'
 
 

src/include/distributed/worker_protocol.h

@@ -42,7 +42,6 @@
 #define MIN_TASK_FILENAME_WIDTH 6
 #define MIN_PARTITION_FILENAME_WIDTH 5
 #define FOREIGN_FILENAME_OPTION "filename"
-#define CSTORE_TABLE_SIZE_FUNCTION_NAME "cstore_table_size"
 
 /* Defines used for fetching files and tables */
 /* the tablename in the overloaded COPY statement is the to-be-transferred file */

src/test/regress/expected/create.out

@@ -1,6 +0,0 @@
-Parsed test spec with 1 sessions
-
-starting permutation: s1a
-step s1a:
-    CREATE EXTENSION cstore_fdw;
-

src/test/regress/expected/multi_extension.out

@@ -899,6 +899,14 @@ SELECT * FROM multi_extension.print_extension_changes();
                  | function worker_fix_partition_shard_index_names(regclass,text,text) void
 (3 rows)
 
+-- Use a synthetic pg_dist_shard record to show that upgrade fails
+-- when there are cstore_fdw tables
+INSERT INTO pg_dist_shard (logicalrelid, shardid, shardstorage) VALUES ('pg_dist_shard', 1, 'c');
+ALTER EXTENSION citus UPDATE TO '11.0-1';
+ERROR:  cstore_fdw tables are deprecated as of Citus 11.0
+HINT:  Install Citus 10.2 and convert your cstore_fdw tables to the columnar access method before upgrading further
+CONTEXT:  PL/pgSQL function inline_code_block line XX at RAISE
+DELETE FROM pg_dist_shard WHERE shardid = 1;
 -- Test downgrade to 10.2-4 from 11.0-1
 ALTER EXTENSION citus UPDATE TO '11.0-1';
 ALTER EXTENSION citus UPDATE TO '10.2-4';

src/test/regress/spec/create.spec

@@ -1,7 +0,0 @@
-session "s1"
-step "s1a"
-{
-    CREATE EXTENSION cstore_fdw;
-}
-
-permutation "s1a"

src/test/regress/sql/multi_extension.sql

@@ -376,6 +376,12 @@ SELECT * FROM multi_extension.print_extension_changes();
 ALTER EXTENSION citus UPDATE TO '10.2-4';
 SELECT * FROM multi_extension.print_extension_changes();
 
+-- Use a synthetic pg_dist_shard record to show that upgrade fails
+-- when there are cstore_fdw tables
+INSERT INTO pg_dist_shard (logicalrelid, shardid, shardstorage) VALUES ('pg_dist_shard', 1, 'c');
+ALTER EXTENSION citus UPDATE TO '11.0-1';
+DELETE FROM pg_dist_shard WHERE shardid = 1;
+
 -- Test downgrade to 10.2-4 from 11.0-1
 ALTER EXTENSION citus UPDATE TO '11.0-1';
 ALTER EXTENSION citus UPDATE TO '10.2-4';