From 010cbf16fc3908bd6aa7cca69bd0c26999bf16a2 Mon Sep 17 00:00:00 2001 From: Robin Thomas Date: Fri, 19 Aug 2016 10:06:22 -0400 Subject: [PATCH] Remove all usage of pg_dist_shard.shardalias in extension code. (#739) Remove regression test of non-null shardalias. --- .../master/master_delete_protocol.c | 17 +---- .../master/master_metadata_utility.c | 73 +------------------ .../master/master_stage_protocol.c | 27 +++---- .../planner/multi_physical_planner.c | 45 +++--------- .../distributed/master_metadata_utility.h | 1 - src/include/distributed/pg_dist_shard.h | 4 +- .../multi_verify_no_join_with_alias.out | 22 ------ src/test/regress/multi_schedule | 1 - .../regress/multi_task_tracker_extra_schedule | 1 - .../sql/multi_verify_no_join_with_alias.sql | 27 ------- 10 files changed, 26 insertions(+), 192 deletions(-) delete mode 100644 src/test/regress/expected/multi_verify_no_join_with_alias.out delete mode 100644 src/test/regress/sql/multi_verify_no_join_with_alias.sql diff --git a/src/backend/distributed/master/master_delete_protocol.c b/src/backend/distributed/master/master_delete_protocol.c index 967be29fe..bb9a9eda5 100644 --- a/src/backend/distributed/master/master_delete_protocol.c +++ b/src/backend/distributed/master/master_delete_protocol.c @@ -24,7 +24,6 @@ #include "access/xact.h" #include "catalog/namespace.h" #include "commands/dbcommands.h" -#include "distributed/master_metadata_utility.h" #include "distributed/master_protocol.h" #include "distributed/multi_client_executor.h" #include "distributed/multi_join_order.h" @@ -319,24 +318,14 @@ DropShards(Oid relationId, char *schemaName, char *relationName, ListCell *lingeringPlacementCell = NULL; ShardInterval *shardInterval = (ShardInterval *) lfirst(shardIntervalCell); uint64 shardId = shardInterval->shardId; - char *shardAlias = NULL; char *quotedShardName = NULL; StringInfo shardName = makeStringInfo(); Assert(shardInterval->relationId == relationId); - /* if shard doesn't have an alias, extend regular table name */ - shardAlias = LoadShardAlias(relationId, shardId); - if (shardAlias == NULL) - { - appendStringInfoString(shardName, relationName); - AppendShardIdToStringInfo(shardName, shardId); - } - else - { - appendStringInfoString(shardName, shardAlias); - } - + /* Build shard relation name. */ + appendStringInfoString(shardName, relationName); + AppendShardIdToStringInfo(shardName, shardId); quotedShardName = quote_qualified_identifier(schemaName, shardName->data); shardPlacementList = ShardPlacementList(shardId); diff --git a/src/backend/distributed/master/master_metadata_utility.c b/src/backend/distributed/master/master_metadata_utility.c index 97faa6518..7244119f7 100644 --- a/src/backend/distributed/master/master_metadata_utility.c +++ b/src/backend/distributed/master/master_metadata_utility.c @@ -138,72 +138,6 @@ AllocateUint64(uint64 value) } -/* - * LoadShardAlias finds the row for given relation and shardId in pg_dist_shard, - * finds the shard alias in this row if any, and then deep copies this alias. - */ -char * -LoadShardAlias(Oid relationId, uint64 shardId) -{ - SysScanDesc scanDescriptor = NULL; - ScanKeyData scanKey[1]; - int scanKeyCount = 1; - HeapTuple heapTuple = NULL; - Datum shardAliasDatum = 0; - bool shardAliasNull = false; - char *shardAlias = NULL; - - Relation pgDistShard = heap_open(DistShardRelationId(), AccessShareLock); - TupleDesc tupleDescriptor = RelationGetDescr(pgDistShard); - - ScanKeyInit(&scanKey[0], Anum_pg_dist_shard_shardid, - BTEqualStrategyNumber, F_INT8EQ, Int64GetDatum(shardId)); - - scanDescriptor = systable_beginscan(pgDistShard, - DistShardShardidIndexId(), true, - NULL, scanKeyCount, scanKey); - - /* - * Normally, we should have at most one tuple here as we have a unique index - * on shardId. However, if users want to drop this uniqueness constraint, - * and look up the shardalias based on the relation and shardId pair, we - * still allow that. We don't have any users relaying on this feature. Thus, - * we may consider to remove this check. - */ - heapTuple = systable_getnext(scanDescriptor); - while (HeapTupleIsValid(heapTuple)) - { - Form_pg_dist_shard pgDistShardForm = (Form_pg_dist_shard) GETSTRUCT(heapTuple); - if (pgDistShardForm->logicalrelid == relationId) - { - break; - } - - heapTuple = systable_getnext(scanDescriptor); - } - - /* if no tuple found, error out */ - if (!HeapTupleIsValid(heapTuple)) - { - ereport(ERROR, (errmsg("could not find valid entry for relationId: %u " - "and shard " UINT64_FORMAT, relationId, shardId))); - } - - /* if shard alias exists, deep copy cstring */ - shardAliasDatum = heap_getattr(heapTuple, Anum_pg_dist_shard_shardalias, - tupleDescriptor, &shardAliasNull); - if (!shardAliasNull) - { - shardAlias = TextDatumGetCString(shardAliasDatum); - } - - systable_endscan(scanDescriptor); - heap_close(pgDistShard, AccessShareLock); - - return shardAlias; -} - - /* * CopyShardInterval copies fields from the specified source ShardInterval * into the fields of the provided destination ShardInterval. @@ -400,20 +334,19 @@ InsertShardRow(Oid relationId, uint64 shardId, char storageType, values[Anum_pg_dist_shard_shardid - 1] = Int64GetDatum(shardId); values[Anum_pg_dist_shard_shardstorage - 1] = CharGetDatum(storageType); + /* deprecated shardalias column is always null. */ + isNulls[Anum_pg_dist_shard_shardalias_DEPRECATED - 1] = true; + /* check if shard min/max values are null */ if (shardMinValue != NULL && shardMaxValue != NULL) { values[Anum_pg_dist_shard_shardminvalue - 1] = PointerGetDatum(shardMinValue); values[Anum_pg_dist_shard_shardmaxvalue - 1] = PointerGetDatum(shardMaxValue); - - /* we always set shard alias to null */ - isNulls[Anum_pg_dist_shard_shardalias - 1] = true; } else { isNulls[Anum_pg_dist_shard_shardminvalue - 1] = true; isNulls[Anum_pg_dist_shard_shardmaxvalue - 1] = true; - isNulls[Anum_pg_dist_shard_shardalias - 1] = true; } /* open shard relation and insert new tuple */ diff --git a/src/backend/distributed/master/master_stage_protocol.c b/src/backend/distributed/master/master_stage_protocol.c index 49941cc8f..62d6fd44a 100644 --- a/src/backend/distributed/master/master_stage_protocol.c +++ b/src/backend/distributed/master/master_stage_protocol.c @@ -238,13 +238,9 @@ master_append_table_to_shard(PG_FUNCTION_ARGS) shardSchemaOid = get_rel_namespace(relationId); shardSchemaName = get_namespace_name(shardSchemaOid); - /* if shard doesn't have an alias, extend regular table name */ - shardTableName = LoadShardAlias(relationId, shardId); - if (shardTableName == NULL) - { - shardTableName = get_rel_name(relationId); - AppendShardIdToName(&shardTableName, shardId); - } + /* Build shard table name. */ + shardTableName = get_rel_name(relationId); + AppendShardIdToName(&shardTableName, shardId); shardQualifiedName = quote_qualified_identifier(shardSchemaName, shardTableName); @@ -493,19 +489,14 @@ UpdateShardStatistics(int64 shardId) text *minValue = NULL; text *maxValue = NULL; - /* if shard doesn't have an alias, extend regular table name */ - shardQualifiedName = LoadShardAlias(relationId, shardId); - if (shardQualifiedName == NULL) - { - char *shardName = get_rel_name(relationId); + /* Build shard qualified name. */ + char *shardName = get_rel_name(relationId); + Oid schemaId = get_rel_namespace(relationId); + char *schemaName = get_namespace_name(schemaId); - Oid schemaId = get_rel_namespace(relationId); - char *schemaName = get_namespace_name(schemaId); + AppendShardIdToName(&shardName, shardId); - AppendShardIdToName(&shardName, shardId); - - shardQualifiedName = quote_qualified_identifier(schemaName, shardName); - } + shardQualifiedName = quote_qualified_identifier(schemaName, shardName); shardPlacementList = FinalizedShardPlacementList(shardId); diff --git a/src/backend/distributed/planner/multi_physical_planner.c b/src/backend/distributed/planner/multi_physical_planner.c index 071b2d2d8..a3d88acf8 100644 --- a/src/backend/distributed/planner/multi_physical_planner.c +++ b/src/backend/distributed/planner/multi_physical_planner.c @@ -31,7 +31,6 @@ #include "distributed/citus_nodefuncs.h" #include "distributed/citus_nodes.h" #include "distributed/citus_ruleutils.h" -#include "distributed/master_metadata_utility.h" #include "distributed/master_protocol.h" #include "distributed/metadata_cache.h" #include "distributed/multi_logical_optimizer.h" @@ -3753,27 +3752,13 @@ ShardFetchQueryString(uint64 shardId) char *shardSchemaName = NULL; char *shardTableName = NULL; - /* - * If user specified a shard alias in pg_dist_shard, error out and display a - * message explaining the limitation. - */ - char *shardAliasName = LoadShardAlias(shardInterval->relationId, shardId); - if (shardAliasName != NULL) - { - ereport(ERROR, (errmsg("cannot fetch shard " UINT64_FORMAT, shardId), - errdetail("Fetching shards with aliases is currently " - "unsupported"))); - } - else - { - /* construct the shard name */ - Oid shardSchemaId = get_rel_namespace(shardInterval->relationId); - char *tableName = get_rel_name(shardInterval->relationId); + /* construct the shard name */ + Oid shardSchemaId = get_rel_namespace(shardInterval->relationId); + char *tableName = get_rel_name(shardInterval->relationId); - shardSchemaName = get_namespace_name(shardSchemaId); - shardTableName = pstrdup(tableName); - AppendShardIdToName(&shardTableName, shardId); - } + shardSchemaName = get_namespace_name(shardSchemaId); + shardTableName = pstrdup(tableName); + AppendShardIdToName(&shardTableName, shardId); shardFetchQuery = makeStringInfo(); if (storageType == SHARD_STORAGE_TABLE || storageType == SHARD_STORAGE_RELAY || @@ -3965,7 +3950,6 @@ FragmentAlias(RangeTblEntry *rangeTableEntry, RangeTableFragment *fragment) CitusRTEKind fragmentType = fragment->fragmentType; if (fragmentType == CITUS_RTE_RELATION) { - char *shardAliasName = NULL; ShardInterval *shardInterval = (ShardInterval *) fragment->fragmentReference; uint64 shardId = shardInterval->shardId; @@ -3986,21 +3970,10 @@ FragmentAlias(RangeTblEntry *rangeTableEntry, RangeTableFragment *fragment) aliasName = relationName; /* - * If user specified a shard name in pg_dist_shard, use that name in alias. - * Otherwise, set shard name in alias to _. + * Set shard name in alias to _. */ - shardAliasName = LoadShardAlias(relationId, shardId); - if (shardAliasName != NULL) - { - fragmentName = shardAliasName; - } - else - { - char *shardName = pstrdup(relationName); - AppendShardIdToName(&shardName, shardId); - - fragmentName = shardName; - } + fragmentName = pstrdup(relationName); + AppendShardIdToName(&fragmentName, shardId); } else if (fragmentType == CITUS_RTE_REMOTE_QUERY) { diff --git a/src/include/distributed/master_metadata_utility.h b/src/include/distributed/master_metadata_utility.h index 3df148fbc..d7610d310 100644 --- a/src/include/distributed/master_metadata_utility.h +++ b/src/include/distributed/master_metadata_utility.h @@ -59,7 +59,6 @@ typedef struct ShardPlacement extern List * LoadShardIntervalList(Oid relationId); extern int ShardIntervalCount(Oid relationId); extern List * LoadShardList(Oid relationId); -extern char * LoadShardAlias(Oid relationId, uint64 shardId); extern void CopyShardInterval(ShardInterval *srcInterval, ShardInterval *destInterval); extern uint64 ShardLength(uint64 shardId); extern List * FinalizedShardPlacementList(uint64 shardId); diff --git a/src/include/distributed/pg_dist_shard.h b/src/include/distributed/pg_dist_shard.h index 5a1dab298..b0e4a4401 100644 --- a/src/include/distributed/pg_dist_shard.h +++ b/src/include/distributed/pg_dist_shard.h @@ -26,7 +26,7 @@ typedef struct FormData_pg_dist_shard int64 shardid; /* global shardId representing remote partition */ char shardstorage; /* shard storage type; see codes below */ #ifdef CATALOG_VARLEN /* variable-length fields start here */ - text shardalias; /* user specified table name for shard, if any */ + text shardalias_DEPRECATED; /* deprecated column, should be unused */ text shardminvalue; /* partition key's minimum value in shard */ text shardmaxvalue; /* partition key's maximum value in shard */ #endif @@ -47,7 +47,7 @@ typedef FormData_pg_dist_shard *Form_pg_dist_shard; #define Anum_pg_dist_shard_logicalrelid 1 #define Anum_pg_dist_shard_shardid 2 #define Anum_pg_dist_shard_shardstorage 3 -#define Anum_pg_dist_shard_shardalias 4 +#define Anum_pg_dist_shard_shardalias_DEPRECATED 4 #define Anum_pg_dist_shard_shardminvalue 5 #define Anum_pg_dist_shard_shardmaxvalue 6 diff --git a/src/test/regress/expected/multi_verify_no_join_with_alias.out b/src/test/regress/expected/multi_verify_no_join_with_alias.out deleted file mode 100644 index 0e6e0df20..000000000 --- a/src/test/regress/expected/multi_verify_no_join_with_alias.out +++ /dev/null @@ -1,22 +0,0 @@ --- --- MULTI_VERIFY_NO_JOIN_WITH_ALIAS --- --- This test checks that we simply emit an error message instead of trying to --- fetch and join a shard which has an alias set. -ALTER SEQUENCE pg_catalog.pg_dist_shardid_seq RESTART 1020000; -ALTER SEQUENCE pg_catalog.pg_dist_jobid_seq RESTART 1020000; --- Show that the join works without an alias -SELECT COUNT(*) FROM lineitem, part WHERE l_partkey = p_partkey; - count -------- - 61 -(1 row) - --- Assign an alias to the parts shard -UPDATE pg_dist_shard SET shardalias = 'my_alias' WHERE shardid = 290000; --- Attempt a join which uses this shard -SELECT COUNT(*) FROM lineitem, part WHERE l_partkey = p_partkey; -ERROR: cannot fetch shard 290000 -DETAIL: Fetching shards with aliases is currently unsupported --- Remove the alias from the parts shard -UPDATE pg_dist_shard SET shardalias = NULL WHERE shardid = 290000; diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index 3f159ee7c..7ea2e94d6 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -44,7 +44,6 @@ test: multi_query_directory_cleanup test: multi_task_assignment_policy test: multi_utility_statements test: multi_dropped_column_aliases -test: multi_verify_no_join_with_alias test: multi_binary_master_copy_format test: multi_prepare_sql multi_prepare_plsql diff --git a/src/test/regress/multi_task_tracker_extra_schedule b/src/test/regress/multi_task_tracker_extra_schedule index 27276f83c..7482d4f42 100644 --- a/src/test/regress/multi_task_tracker_extra_schedule +++ b/src/test/regress/multi_task_tracker_extra_schedule @@ -39,7 +39,6 @@ test: multi_query_directory_cleanup test: multi_task_assignment_policy test: multi_utility_statements test: multi_dropped_column_aliases -test: multi_verify_no_join_with_alias # ---------- # Parallel TPC-H tests to check our distributed execution behavior diff --git a/src/test/regress/sql/multi_verify_no_join_with_alias.sql b/src/test/regress/sql/multi_verify_no_join_with_alias.sql deleted file mode 100644 index 18e446df1..000000000 --- a/src/test/regress/sql/multi_verify_no_join_with_alias.sql +++ /dev/null @@ -1,27 +0,0 @@ --- --- MULTI_VERIFY_NO_JOIN_WITH_ALIAS --- - --- This test checks that we simply emit an error message instead of trying to --- fetch and join a shard which has an alias set. - - -ALTER SEQUENCE pg_catalog.pg_dist_shardid_seq RESTART 1020000; -ALTER SEQUENCE pg_catalog.pg_dist_jobid_seq RESTART 1020000; - - --- Show that the join works without an alias - -SELECT COUNT(*) FROM lineitem, part WHERE l_partkey = p_partkey; - --- Assign an alias to the parts shard - -UPDATE pg_dist_shard SET shardalias = 'my_alias' WHERE shardid = 290000; - --- Attempt a join which uses this shard - -SELECT COUNT(*) FROM lineitem, part WHERE l_partkey = p_partkey; - --- Remove the alias from the parts shard - -UPDATE pg_dist_shard SET shardalias = NULL WHERE shardid = 290000;