From 52f11223e51f9d686014033c12736accbb8d1d7b Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Thu, 11 Feb 2016 20:42:23 +0100 Subject: [PATCH 1/5] Drop shards when a distributed table is dropped After this change, shards and associated metadata are automatically dropped when running DROP TABLE on a distributed table, which fixes #230. It also adds schema support for master_apply_delete_command, which fixes #73. Dropping the shards happens in the master_drop_all_shards UDF, which is called from the SQL_DROP trigger. Inside the trigger, the table is no longer visible and calling master_apply_delete_command directly wouldn't work and oid <-> name mappings are not available. The master_drop_all_shards function therefore takes the relation id, schema name, and table name as parameters, which can be obtained from pg_event_trigger_dropped_objects() in the SQL_DROP trigger. If the user calls master_drop_all_shards while the table still exists, the schema name and table name are ignored. Author: Marco Slot Reviewed-By: Andres Freund --- src/backend/distributed/citusdb.sql | 24 ++-- .../master/master_delete_protocol.c | 120 +++++++++++++++--- .../master/master_metadata_utility.c | 17 ++- .../distributed/relay/relay_event_utility.c | 12 ++ src/include/distributed/master_protocol.h | 1 + src/include/distributed/relay_utility.h | 2 + .../expected/multi_index_statements_0.out | 8 +- src/test/regress/expected/multi_table_ddl.out | 32 ++--- src/test/regress/expected/multi_utilities.out | 4 - .../input/multi_master_delete_protocol.source | 5 + .../multi_alter_table_statements.source | 16 +-- .../multi_alter_table_statements_0.source | 38 +++--- .../multi_master_delete_protocol.source | 5 + src/test/regress/sql/multi_table_ddl.sql | 17 ++- src/test/regress/sql/multi_utilities.sql | 3 - 15 files changed, 213 insertions(+), 91 deletions(-) diff --git a/src/backend/distributed/citusdb.sql b/src/backend/distributed/citusdb.sql index 1090d5bd4..c2bb330e5 100644 --- a/src/backend/distributed/citusdb.sql +++ b/src/backend/distributed/citusdb.sql @@ -148,6 +148,15 @@ CREATE FUNCTION master_append_table_to_shard(bigint, text, text, integer) COMMENT ON FUNCTION master_append_table_to_shard(bigint, text, text, integer) IS 'append given table to all shard placements and update metadata'; +CREATE FUNCTION master_drop_all_shards(logicalrelid regclass, + schema_name text, + table_name text) + RETURNS integer + LANGUAGE C STRICT + AS 'MODULE_PATHNAME', $$master_drop_all_shards$$; +COMMENT ON FUNCTION master_drop_all_shards(regclass, text, text) + IS 'drop all shards in a relation and update metadata'; + CREATE FUNCTION master_apply_delete_command(text) RETURNS integer LANGUAGE C STRICT @@ -322,7 +331,7 @@ CREATE OR REPLACE FUNCTION citusdb_drop_trigger() DECLARE v_obj record; BEGIN FOR v_obj IN SELECT * FROM pg_event_trigger_dropped_objects() LOOP - IF v_obj.object_type <> 'table' THEN + IF v_obj.object_type NOT IN ('table', 'foreign table') THEN CONTINUE; END IF; @@ -331,20 +340,11 @@ BEGIN CONTINUE; END IF; - -- check if there's shards for the table, error out if so - IF EXISTS(SELECT * FROM pg_dist_shard WHERE logicalrelid = v_obj.objid) THEN - RAISE EXCEPTION USING - MESSAGE = 'cannot drop distributed table with existing shards', - HINT = $$Delete shards first using: $$ || - $$SELECT master_apply_delete_command('DELETE FROM $$ || - v_obj.object_identity || $$')$$; - END IF; + -- ensure all shards are dropped + PERFORM master_drop_all_shards(v_obj.objid, v_obj.schema_name, v_obj.object_name); -- delete partition entry DELETE FROM pg_dist_partition WHERE logicalrelid = v_obj.objid; - IF NOT FOUND THEN - RAISE EXCEPTION 'could not find previously found pg_dist_partition entry'; - END IF; END LOOP; END; diff --git a/src/backend/distributed/master/master_delete_protocol.c b/src/backend/distributed/master/master_delete_protocol.c index 35835c7d0..9d0c6355e 100644 --- a/src/backend/distributed/master/master_delete_protocol.c +++ b/src/backend/distributed/master/master_delete_protocol.c @@ -18,20 +18,25 @@ #include "funcapi.h" #include "miscadmin.h" +#include "access/xact.h" +#include "catalog/namespace.h" #include "catalog/pg_class.h" #include "commands/dbcommands.h" +#include "commands/event_trigger.h" #include "distributed/master_metadata_utility.h" #include "distributed/master_protocol.h" #include "distributed/metadata_cache.h" #include "distributed/multi_client_executor.h" #include "distributed/multi_physical_planner.h" #include "distributed/multi_server_executor.h" +#include "distributed/pg_dist_shard.h" #include "distributed/pg_dist_partition.h" #include "distributed/worker_protocol.h" #include "optimizer/clauses.h" #include "optimizer/predtest.h" #include "optimizer/restrictinfo.h" #include "optimizer/var.h" +#include "nodes/makefuncs.h" #include "tcop/tcopprot.h" #include "utils/builtins.h" #include "utils/datum.h" @@ -45,12 +50,15 @@ static void CheckDeleteCriteria(Node *deleteCriteria); static void CheckPartitionColumn(Oid relationId, Node *whereClause); static List * ShardsMatchingDeleteCriteria(Oid relationId, List *shardList, Node *deleteCriteria); +static int DropShards(Oid relationId, char *schemaName, char *relationName, + List *deletableShardIntervalList); static bool ExecuteRemoteCommand(const char *nodeName, uint32 nodePort, StringInfo queryString); /* exports for SQL callable functions */ PG_FUNCTION_INFO_V1(master_apply_delete_command); +PG_FUNCTION_INFO_V1(master_drop_all_shards); /* @@ -72,10 +80,9 @@ master_apply_delete_command(PG_FUNCTION_ARGS) text *queryText = PG_GETARG_TEXT_P(0); char *queryString = text_to_cstring(queryText); char *relationName = NULL; - text *relationNameText = NULL; + char *schemaName = NULL; Oid relationId = InvalidOid; List *shardIntervalList = NIL; - ListCell *shardIntervalCell = NULL; List *deletableShardIntervalList = NIL; List *queryTreeList = NIL; Query *deleteQuery = NULL; @@ -83,11 +90,15 @@ master_apply_delete_command(PG_FUNCTION_ARGS) Node *deleteCriteria = NULL; Node *queryTreeNode = NULL; DeleteStmt *deleteStatement = NULL; - int32 deleteCriteriaShardCount = 0; + int droppedShardCount = 0; LOCKTAG lockTag; bool sessionLock = false; bool dontWait = false; char partitionMethod = 0; + bool failOK = false; + bool topLevel = true; + + PreventTransactionChain(topLevel, "master_apply_delete_command"); queryTreeNode = ParseTreeNode(queryString); if (!IsA(queryTreeNode, DeleteStmt)) @@ -97,10 +108,10 @@ master_apply_delete_command(PG_FUNCTION_ARGS) } deleteStatement = (DeleteStmt *) queryTreeNode; - relationName = deleteStatement->relation->relname; - relationNameText = cstring_to_text(relationName); - relationId = ResolveRelationId(relationNameText); + schemaName = deleteStatement->relation->schemaname; + relationName = deleteStatement->relation->relname; + relationId = RangeVarGetRelid(deleteStatement->relation, NoLock, failOK); CheckDistributedTable(relationId); queryTreeList = pg_analyze_and_rewrite(queryTreeNode, queryString, NULL, 0); @@ -142,6 +153,71 @@ master_apply_delete_command(PG_FUNCTION_ARGS) deleteCriteria); } + droppedShardCount = DropShards(relationId, schemaName, relationName, + deletableShardIntervalList); + + PG_RETURN_INT32(droppedShardCount); +} + + +/* + * master_drop_shards attempts to drop all shards for a given relation. + * Unlike master_apply_delete_command, this function can be called even + * if the table has already been dropped. + */ +Datum +master_drop_all_shards(PG_FUNCTION_ARGS) +{ + Oid relationId = PG_GETARG_OID(0); + text *schemaNameText = PG_GETARG_TEXT_P(1); + text *relationNameText = PG_GETARG_TEXT_P(2); + + char *schemaName = NULL; + char *relationName = NULL; + bool topLevel = true; + List *shardIntervalList = NIL; + int droppedShardCount = 0; + + PreventTransactionChain(topLevel, "DROP distributed table"); + + relationName = get_rel_name(relationId); + + if (relationName != NULL) + { + /* ensure proper values are used if the table exists */ + Oid schemaId = get_rel_namespace(relationId); + schemaName = get_namespace_name(schemaId); + } + else + { + /* table has been dropped, rely on user-supplied values */ + schemaName = text_to_cstring(schemaNameText); + relationName = text_to_cstring(relationNameText); + } + + shardIntervalList = LoadShardIntervalList(relationId); + droppedShardCount = DropShards(relationId, schemaName, relationName, + shardIntervalList); + + PG_RETURN_INT32(droppedShardCount); +} + + +/* + * DropShards drops all given shards in a relation. The id, name and schema + * for the relation are explicitly provided, since this function may be + * called when the table is already dropped. + * + * We mark shard placements that we couldn't drop as to be deleted later, but + * we do delete the shard metadadata. + */ +int +DropShards(Oid relationId, char *schemaName, char *relationName, + List *deletableShardIntervalList) +{ + ListCell *shardIntervalCell = NULL; + int droppedShardCount = 0; + foreach(shardIntervalCell, deletableShardIntervalList) { List *shardPlacementList = NIL; @@ -152,17 +228,25 @@ master_apply_delete_command(PG_FUNCTION_ARGS) 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 */ - char *shardName = LoadShardAlias(relationId, shardId); - if (shardName == NULL) + shardAlias = LoadShardAlias(relationId, shardId); + if (shardAlias == NULL) { - shardName = get_rel_name(relationId); - AppendShardIdToName(&shardName, shardId); + appendStringInfoString(shardName, relationName); + AppendShardIdToStringInfo(shardName, shardId); + } + else + { + appendStringInfoString(shardName, shardAlias); } - quotedShardName = quote_qualified_identifier(NULL, shardName); + quotedShardName = quote_qualified_identifier(schemaName, shardName->data); shardPlacementList = ShardPlacementList(shardId); foreach(shardPlacementCell, shardPlacementList) @@ -173,12 +257,13 @@ master_apply_delete_command(PG_FUNCTION_ARGS) bool dropSuccessful = false; StringInfo workerDropQuery = makeStringInfo(); - char tableType = get_rel_relkind(relationId); - if (tableType == RELKIND_RELATION) + char storageType = shardInterval->storageType; + if (storageType == SHARD_STORAGE_TABLE) { appendStringInfo(workerDropQuery, DROP_REGULAR_TABLE_COMMAND, quotedShardName); } - else if (tableType == RELKIND_FOREIGN_TABLE) + else if (storageType == SHARD_STORAGE_COLUMNAR || + storageType == SHARD_STORAGE_FOREIGN) { appendStringInfo(workerDropQuery, DROP_FOREIGN_TABLE_COMMAND, quotedShardName); } @@ -219,7 +304,7 @@ master_apply_delete_command(PG_FUNCTION_ARGS) workerName, workerPort); ereport(WARNING, (errmsg("could not delete shard \"%s\" on node " - "\"%s:%u\"", shardName, workerName, workerPort), + "\"%s:%u\"", shardName->data, workerName, workerPort), errdetail("Marking this shard placement for deletion"))); } @@ -234,8 +319,9 @@ master_apply_delete_command(PG_FUNCTION_ARGS) RESUME_INTERRUPTS(); } - deleteCriteriaShardCount = list_length(deletableShardIntervalList); - PG_RETURN_INT32(deleteCriteriaShardCount); + droppedShardCount = list_length(deletableShardIntervalList); + + return droppedShardCount; } diff --git a/src/backend/distributed/master/master_metadata_utility.c b/src/backend/distributed/master/master_metadata_utility.c index d8ac90997..20eb70028 100644 --- a/src/backend/distributed/master/master_metadata_utility.c +++ b/src/backend/distributed/master/master_metadata_utility.c @@ -452,6 +452,10 @@ DeleteShardRow(uint64 shardId) HeapTuple heapTuple = NULL; Form_pg_dist_shard pgDistShardForm = NULL; Oid distributedRelationId = InvalidOid; + HeapTuple relationOidTuple = NULL; + TupleDesc tupleDescriptor = NULL; + Datum tupleValues[1] = { (Datum) NULL }; + bool tupleNulls[1] = { false }; pgDistShard = heap_open(DistShardRelationId(), RowExclusiveLock); @@ -478,8 +482,17 @@ DeleteShardRow(uint64 shardId) systable_endscan(scanDescriptor); heap_close(pgDistShard, RowExclusiveLock); - /* invalidate previous cache entry */ - CacheInvalidateRelcacheByRelid(distributedRelationId); + /* + * Invalidate using a heap tuple containing the relation OID. We avoid calling + * CacheInvalidateRelcacheByRelid here, since that throw an error if the table + * is no longer in the catalog, which is the case when calling this function + * from a DROP TABLE trigger. + */ + tupleDescriptor = CreateTemplateTupleDesc(1, true); + TupleDescInitEntry(tupleDescriptor, (AttrNumber) 1, "relation", OIDOID, -1, 0); + tupleValues[0] = ObjectIdGetDatum(distributedRelationId); + relationOidTuple = heap_form_tuple(tupleDescriptor, tupleValues, tupleNulls); + CacheInvalidateRelcacheByTuple(relationOidTuple); } diff --git a/src/backend/distributed/relay/relay_event_utility.c b/src/backend/distributed/relay/relay_event_utility.c index 54f7a09a5..926f4f25e 100644 --- a/src/backend/distributed/relay/relay_event_utility.c +++ b/src/backend/distributed/relay/relay_event_utility.c @@ -509,3 +509,15 @@ AppendShardIdToName(char **name, uint64 shardId) (*name) = (char *) repalloc((*name), extendedNameLength); snprintf((*name), extendedNameLength, "%s", extendedName); } + + +/* + * AppendShardIdToStringInfo appends shardId to the given name, represented + * by a StringInfo. + */ +void +AppendShardIdToStringInfo(StringInfo name, uint64 shardId) +{ + appendStringInfo(name, "%c" UINT64_FORMAT, SHARD_NAME_SEPARATOR, shardId); +} + diff --git a/src/include/distributed/master_protocol.h b/src/include/distributed/master_protocol.h index f39ce865b..ea40538d3 100644 --- a/src/include/distributed/master_protocol.h +++ b/src/include/distributed/master_protocol.h @@ -98,6 +98,7 @@ extern Datum master_get_active_worker_nodes(PG_FUNCTION_ARGS); extern Datum master_create_empty_shard(PG_FUNCTION_ARGS); extern Datum master_append_table_to_shard(PG_FUNCTION_ARGS); extern Datum master_apply_delete_command(PG_FUNCTION_ARGS); +extern Datum master_drop_all_shards(PG_FUNCTION_ARGS); /* function declarations for shard creation functionality */ extern Datum master_create_worker_shards(PG_FUNCTION_ARGS); diff --git a/src/include/distributed/relay_utility.h b/src/include/distributed/relay_utility.h index 592f61632..3685b3559 100644 --- a/src/include/distributed/relay_utility.h +++ b/src/include/distributed/relay_utility.h @@ -15,6 +15,7 @@ #ifndef RELAY_UTILITY_H #define RELAY_UTILITY_H +#include "lib/stringinfo.h" #include "nodes/nodes.h" @@ -42,6 +43,7 @@ typedef enum /* Function declarations to extend names in DDL commands */ extern void RelayEventExtendNames(Node *parseTree, uint64 shardId); extern void AppendShardIdToName(char **name, uint64 shardId); +extern void AppendShardIdToStringInfo(StringInfo name, uint64 shardId); #endif /* RELAY_UTILITY_H */ diff --git a/src/test/regress/expected/multi_index_statements_0.out b/src/test/regress/expected/multi_index_statements_0.out index e7cc610e4..36cf47981 100644 --- a/src/test/regress/expected/multi_index_statements_0.out +++ b/src/test/regress/expected/multi_index_statements_0.out @@ -136,7 +136,7 @@ DEBUG: applied command on shard 102010 on node localhost:57637 DEBUG: applied command on shard 102010 on node localhost:57638 DEBUG: applied command on shard 102009 on node localhost:57637 DEBUG: applied command on shard 102009 on node localhost:57638 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 DROP INDEX lineitem_partkey_desc_index; DEBUG: applied command on shard 102014 on node localhost:57637 DEBUG: applied command on shard 102014 on node localhost:57638 @@ -150,7 +150,7 @@ DEBUG: applied command on shard 102010 on node localhost:57637 DEBUG: applied command on shard 102010 on node localhost:57638 DEBUG: applied command on shard 102009 on node localhost:57637 DEBUG: applied command on shard 102009 on node localhost:57638 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 DROP INDEX lineitem_partial_index; DEBUG: applied command on shard 102014 on node localhost:57637 DEBUG: applied command on shard 102014 on node localhost:57638 @@ -164,7 +164,7 @@ DEBUG: applied command on shard 102010 on node localhost:57637 DEBUG: applied command on shard 102010 on node localhost:57638 DEBUG: applied command on shard 102009 on node localhost:57637 DEBUG: applied command on shard 102009 on node localhost:57638 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 -- Verify that we handle if exists statements correctly DROP INDEX non_existent_index; ERROR: index "non_existent_index" does not exist @@ -183,7 +183,7 @@ DEBUG: applied command on shard 102010 on node localhost:57637 DEBUG: applied command on shard 102010 on node localhost:57638 DEBUG: applied command on shard 102009 on node localhost:57637 DEBUG: applied command on shard 102009 on node localhost:57638 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 DROP INDEX lineitem_orderkey_hash_index; ERROR: index "lineitem_orderkey_hash_index" does not exist -- Verify that all the indexes are also dropped from the master node diff --git a/src/test/regress/expected/multi_table_ddl.out b/src/test/regress/expected/multi_table_ddl.out index 9c2488fe0..ebf07f786 100644 --- a/src/test/regress/expected/multi_table_ddl.out +++ b/src/test/regress/expected/multi_table_ddl.out @@ -22,27 +22,29 @@ ERROR: cannot execute ALTER TABLE command involving partition column -- verify that the distribution column can't be dropped ALTER TABLE testtableddl DROP COLUMN distributecol; ERROR: cannot execute ALTER TABLE command involving partition column --- verify that the table cannot be dropped while shards exist +-- verify that the table cannot be dropped in a transaction block +BEGIN; +DROP TABLE testtableddl; +ERROR: DROP distributed table cannot run inside a transaction block +CONTEXT: SQL statement "SELECT master_drop_all_shards(v_obj.objid, v_obj.schema_name, v_obj.object_name)" +PL/pgSQL function citusdb_drop_trigger() line 15 at PERFORM +ROLLBACK; +-- verify that the table can be dropped +DROP TABLE testtableddl; +-- verify that the table can dropped even if shards exist +CREATE TABLE testtableddl(somecol int, distributecol text NOT NULL); +SELECT master_create_distributed_table('testtableddl', 'distributecol', 'append'); + master_create_distributed_table +--------------------------------- + +(1 row) + SELECT 1 FROM master_create_empty_shard('testtableddl'); ?column? ---------- 1 (1 row) -DROP TABLE testtableddl; -ERROR: cannot drop distributed table with existing shards -HINT: Delete shards first using: SELECT master_apply_delete_command('DELETE FROM public.testtableddl') --- not even with cascade -DROP TABLE testtableddl CASCADE; -ERROR: cannot drop distributed table with existing shards -HINT: Delete shards first using: SELECT master_apply_delete_command('DELETE FROM public.testtableddl') --- but it can be dropped after dropping the shards -SELECT master_apply_delete_command('DELETE FROM testtableddl'); - master_apply_delete_command ------------------------------ - 1 -(1 row) - DROP TABLE testtableddl; -- ensure no metadata of distributed tables are remaining SELECT * FROM pg_dist_partition; diff --git a/src/test/regress/expected/multi_utilities.out b/src/test/regress/expected/multi_utilities.out index 46d2213eb..6deec6f9f 100644 --- a/src/test/regress/expected/multi_utilities.out +++ b/src/test/regress/expected/multi_utilities.out @@ -65,10 +65,6 @@ EXECUTE sharded_query; ------ (0 rows) --- try to drop table -DROP TABLE sharded_table; -ERROR: cannot drop distributed table with existing shards -HINT: Delete shards first using: SELECT master_apply_delete_command('DELETE FROM public.sharded_table') -- try to drop shards with where clause SELECT master_apply_delete_command('DELETE FROM sharded_table WHERE id > 0'); ERROR: cannot delete from distributed table diff --git a/src/test/regress/input/multi_master_delete_protocol.source b/src/test/regress/input/multi_master_delete_protocol.source index bafece8dd..c98febba4 100644 --- a/src/test/regress/input/multi_master_delete_protocol.source +++ b/src/test/regress/input/multi_master_delete_protocol.source @@ -46,3 +46,8 @@ SELECT master_create_empty_shard('customer_delete_protocol'); SELECT master_apply_delete_command('DELETE FROM customer_delete_protocol WHERE c_custkey > 1000'); SELECT master_apply_delete_command('DELETE FROM customer_delete_protocol'); + +-- Verify that master_apply_delete_command cannot be called in a transaction block +BEGIN; +SELECT master_apply_delete_command('DELETE FROM customer_delete_protocol'); +ROLLBACK; diff --git a/src/test/regress/output/multi_alter_table_statements.source b/src/test/regress/output/multi_alter_table_statements.source index 90b644d72..a5c0d17ef 100644 --- a/src/test/regress/output/multi_alter_table_statements.source +++ b/src/test/regress/output/multi_alter_table_statements.source @@ -55,8 +55,8 @@ DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 DEBUG: rewriting table "lineitem_alter" -DEBUG: drop auto-cascades to type pg_temp_17666 -DEBUG: drop auto-cascades to type pg_temp_17666[] +DEBUG: drop auto-cascades to type pg_temp_17675 +DEBUG: drop auto-cascades to type pg_temp_17675[] ALTER TABLE lineitem_alter ADD COLUMN int_column2 INTEGER DEFAULT 2; DEBUG: applied command on shard 103002 on node localhost:57638 DEBUG: applied command on shard 103002 on node localhost:57637 @@ -65,8 +65,8 @@ DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 DEBUG: rewriting table "lineitem_alter" -DEBUG: drop auto-cascades to type pg_temp_17666 -DEBUG: drop auto-cascades to type pg_temp_17666[] +DEBUG: drop auto-cascades to type pg_temp_17675 +DEBUG: drop auto-cascades to type pg_temp_17675[] ALTER TABLE lineitem_alter ADD COLUMN null_column INTEGER; DEBUG: applied command on shard 103002 on node localhost:57638 DEBUG: applied command on shard 103002 on node localhost:57637 @@ -359,8 +359,8 @@ DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 DEBUG: rewriting table "lineitem_alter" -DEBUG: drop auto-cascades to type pg_temp_17666 -DEBUG: drop auto-cascades to type pg_temp_17666[] +DEBUG: drop auto-cascades to type pg_temp_17675 +DEBUG: drop auto-cascades to type pg_temp_17675[] \d lineitem_alter Table "public.lineitem_alter" Column | Type | Modifiers @@ -478,8 +478,8 @@ DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 DEBUG: rewriting table "lineitem_alter" -DEBUG: drop auto-cascades to type pg_temp_17666 -DEBUG: drop auto-cascades to type pg_temp_17666[] +DEBUG: drop auto-cascades to type pg_temp_17675 +DEBUG: drop auto-cascades to type pg_temp_17675[] ALTER TABLE lineitem_alter DROP COLUMN non_existent_column; WARNING: could not receive query results from localhost:57637 DETAIL: Client error: column "non_existent_column" of relation "lineitem_alter_103009" does not exist diff --git a/src/test/regress/output/multi_alter_table_statements_0.source b/src/test/regress/output/multi_alter_table_statements_0.source index 1e5bd8cc6..0d13a8ee4 100644 --- a/src/test/regress/output/multi_alter_table_statements_0.source +++ b/src/test/regress/output/multi_alter_table_statements_0.source @@ -54,10 +54,10 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerTableRewrite(17667) +DEBUG: EventTriggerTableRewrite(17676) DEBUG: rewriting table "lineitem_alter" -DEBUG: drop auto-cascades to type pg_temp_17667 -DEBUG: drop auto-cascades to type pg_temp_17667[] +DEBUG: drop auto-cascades to type pg_temp_17676 +DEBUG: drop auto-cascades to type pg_temp_17676[] ALTER TABLE lineitem_alter ADD COLUMN int_column2 INTEGER DEFAULT 2; DEBUG: applied command on shard 103002 on node localhost:57638 DEBUG: applied command on shard 103002 on node localhost:57637 @@ -65,10 +65,10 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerTableRewrite(17667) +DEBUG: EventTriggerTableRewrite(17676) DEBUG: rewriting table "lineitem_alter" -DEBUG: drop auto-cascades to type pg_temp_17667 -DEBUG: drop auto-cascades to type pg_temp_17667[] +DEBUG: drop auto-cascades to type pg_temp_17676 +DEBUG: drop auto-cascades to type pg_temp_17676[] ALTER TABLE lineitem_alter ADD COLUMN null_column INTEGER; DEBUG: applied command on shard 103002 on node localhost:57638 DEBUG: applied command on shard 103002 on node localhost:57637 @@ -129,7 +129,7 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 -- \stage to verify that default values take effect \STAGE lineitem_alter (l_orderkey, l_partkey, l_suppkey, l_linenumber, l_quantity, l_extendedprice, l_discount, l_tax, l_returnflag, l_linestatus, l_shipdate, l_commitdate, l_receiptdate, l_shipinstruct, l_shipmode, l_comment) FROM '@abs_srcdir@/data/lineitem.1.data' with delimiter '|' DEBUG: parse : SELECT * FROM master_get_table_metadata($1::text) @@ -235,7 +235,7 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 -- \stage should fail because it will try to insert NULLs for a NOT NULL column \STAGE lineitem_alter (l_orderkey, l_partkey, l_suppkey, l_linenumber, l_quantity, l_extendedprice, l_discount, l_tax, l_returnflag, l_linestatus, l_shipdate, l_commitdate, l_receiptdate, l_shipinstruct, l_shipmode, l_comment) FROM '@abs_srcdir@/data/lineitem.1.data' with delimiter '|' DEBUG: parse : SELECT * FROM master_get_table_metadata($1::text) @@ -362,10 +362,10 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerTableRewrite(17667) +DEBUG: EventTriggerTableRewrite(17676) DEBUG: rewriting table "lineitem_alter" -DEBUG: drop auto-cascades to type pg_temp_17667 -DEBUG: drop auto-cascades to type pg_temp_17667[] +DEBUG: drop auto-cascades to type pg_temp_17676 +DEBUG: drop auto-cascades to type pg_temp_17676[] \d lineitem_alter Table "public.lineitem_alter" Column | Type | Modifiers @@ -419,7 +419,7 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 ALTER TABLE lineitem_alter DROP COLUMN float_column; DEBUG: applied command on shard 103009 on node localhost:57637 DEBUG: applied command on shard 103009 on node localhost:57638 @@ -440,7 +440,7 @@ DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 DEBUG: drop auto-cascades to default for table lineitem_alter column float_column -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 ALTER TABLE lineitem_alter DROP COLUMN date_column; DEBUG: applied command on shard 103009 on node localhost:57637 DEBUG: applied command on shard 103009 on node localhost:57638 @@ -460,7 +460,7 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 -- Verify that IF EXISTS works as expected ALTER TABLE non_existent_table ADD COLUMN new_column INTEGER; ERROR: relation "non_existent_table" does not exist @@ -485,10 +485,10 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerTableRewrite(17667) +DEBUG: EventTriggerTableRewrite(17676) DEBUG: rewriting table "lineitem_alter" -DEBUG: drop auto-cascades to type pg_temp_17667 -DEBUG: drop auto-cascades to type pg_temp_17667[] +DEBUG: drop auto-cascades to type pg_temp_17676 +DEBUG: drop auto-cascades to type pg_temp_17676[] ALTER TABLE lineitem_alter DROP COLUMN non_existent_column; WARNING: could not receive query results from localhost:57637 DETAIL: Client error: column "non_existent_column" of relation "lineitem_alter_103009" does not exist @@ -532,7 +532,7 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 \d lineitem_alter Table "public.lineitem_alter" Column | Type | Modifiers @@ -623,7 +623,7 @@ DEBUG: applied command on shard 103001 on node localhost:57637 DEBUG: applied command on shard 103001 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57638 DEBUG: applied command on shard 103000 on node localhost:57637 -DEBUG: EventTriggerInvoke 16532 +DEBUG: EventTriggerInvoke 16541 \d lineitem_alter Table "public.lineitem_alter" Column | Type | Modifiers diff --git a/src/test/regress/output/multi_master_delete_protocol.source b/src/test/regress/output/multi_master_delete_protocol.source index 1306eb668..84fd2579c 100644 --- a/src/test/regress/output/multi_master_delete_protocol.source +++ b/src/test/regress/output/multi_master_delete_protocol.source @@ -86,3 +86,8 @@ SELECT master_apply_delete_command('DELETE FROM customer_delete_protocol'); 1 (1 row) +-- Verify that master_apply_delete_command cannot be called in a transaction block +BEGIN; +SELECT master_apply_delete_command('DELETE FROM customer_delete_protocol'); +ERROR: master_apply_delete_command cannot run inside a transaction block +ROLLBACK; diff --git a/src/test/regress/sql/multi_table_ddl.sql b/src/test/regress/sql/multi_table_ddl.sql index f2f8a6290..f820ce70d 100644 --- a/src/test/regress/sql/multi_table_ddl.sql +++ b/src/test/regress/sql/multi_table_ddl.sql @@ -15,15 +15,18 @@ ALTER TABLE testtableddl ALTER COLUMN distributecol TYPE text; -- verify that the distribution column can't be dropped ALTER TABLE testtableddl DROP COLUMN distributecol; --- verify that the table cannot be dropped while shards exist -SELECT 1 FROM master_create_empty_shard('testtableddl'); +-- verify that the table cannot be dropped in a transaction block +BEGIN; +DROP TABLE testtableddl; +ROLLBACK; + +-- verify that the table can be dropped DROP TABLE testtableddl; --- not even with cascade -DROP TABLE testtableddl CASCADE; - --- but it can be dropped after dropping the shards -SELECT master_apply_delete_command('DELETE FROM testtableddl'); +-- verify that the table can dropped even if shards exist +CREATE TABLE testtableddl(somecol int, distributecol text NOT NULL); +SELECT master_create_distributed_table('testtableddl', 'distributecol', 'append'); +SELECT 1 FROM master_create_empty_shard('testtableddl'); DROP TABLE testtableddl; -- ensure no metadata of distributed tables are remaining diff --git a/src/test/regress/sql/multi_utilities.sql b/src/test/regress/sql/multi_utilities.sql index 457a79489..a503b2c0d 100644 --- a/src/test/regress/sql/multi_utilities.sql +++ b/src/test/regress/sql/multi_utilities.sql @@ -36,9 +36,6 @@ EXECUTE sharded_query; EXECUTE sharded_delete; EXECUTE sharded_query; --- try to drop table -DROP TABLE sharded_table; - -- try to drop shards with where clause SELECT master_apply_delete_command('DELETE FROM sharded_table WHERE id > 0'); From 2af6797c048de707fe61d32163fb3e7d49a79643 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Tue, 16 Feb 2016 12:59:38 +0100 Subject: [PATCH 2/5] Perform relcache invalidation in CitusInvalidateRelcacheByRelid --- .../commands/create_distributed_table.c | 2 +- .../master/master_metadata_utility.c | 19 +----- .../distributed/utils/metadata_cache.c | 58 +++++++++---------- src/include/distributed/metadata_cache.h | 1 + 4 files changed, 31 insertions(+), 49 deletions(-) diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index b4a4c802b..7eabbf4fc 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -246,7 +246,7 @@ master_create_distributed_table(PG_FUNCTION_ARGS) /* finally insert tuple, build index entries & register cache invalidation */ simple_heap_insert(pgDistPartition, newTuple); CatalogUpdateIndexes(pgDistPartition, newTuple); - CacheInvalidateRelcacheByRelid(distributedRelationId); + CitusInvalidateRelcacheByRelid(distributedRelationId); RecordDistributedRelationDependencies(distributedRelationId, distributionKey); diff --git a/src/backend/distributed/master/master_metadata_utility.c b/src/backend/distributed/master/master_metadata_utility.c index 20eb70028..18ac2bce0 100644 --- a/src/backend/distributed/master/master_metadata_utility.c +++ b/src/backend/distributed/master/master_metadata_utility.c @@ -394,7 +394,7 @@ InsertShardRow(Oid relationId, uint64 shardId, char storageType, /* close relation and invalidate previous cache entry */ heap_close(pgDistShard, RowExclusiveLock); - CacheInvalidateRelcacheByRelid(relationId); + CitusInvalidateRelcacheByRelid(relationId); } @@ -452,10 +452,6 @@ DeleteShardRow(uint64 shardId) HeapTuple heapTuple = NULL; Form_pg_dist_shard pgDistShardForm = NULL; Oid distributedRelationId = InvalidOid; - HeapTuple relationOidTuple = NULL; - TupleDesc tupleDescriptor = NULL; - Datum tupleValues[1] = { (Datum) NULL }; - bool tupleNulls[1] = { false }; pgDistShard = heap_open(DistShardRelationId(), RowExclusiveLock); @@ -482,17 +478,8 @@ DeleteShardRow(uint64 shardId) systable_endscan(scanDescriptor); heap_close(pgDistShard, RowExclusiveLock); - /* - * Invalidate using a heap tuple containing the relation OID. We avoid calling - * CacheInvalidateRelcacheByRelid here, since that throw an error if the table - * is no longer in the catalog, which is the case when calling this function - * from a DROP TABLE trigger. - */ - tupleDescriptor = CreateTemplateTupleDesc(1, true); - TupleDescInitEntry(tupleDescriptor, (AttrNumber) 1, "relation", OIDOID, -1, 0); - tupleValues[0] = ObjectIdGetDatum(distributedRelationId); - relationOidTuple = heap_form_tuple(tupleDescriptor, tupleValues, tupleNulls); - CacheInvalidateRelcacheByTuple(relationOidTuple); + /* invalidate previous cache entry */ + CitusInvalidateRelcacheByRelid(distributedRelationId); } diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index 868b11313..b670f0384 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -484,26 +484,12 @@ master_dist_partition_cache_invalidate(PG_FUNCTION_ARGS) if (oldLogicalRelationId != InvalidOid && oldLogicalRelationId != newLogicalRelationId) { - HeapTuple oldClassTuple = - SearchSysCache1(RELOID, ObjectIdGetDatum(oldLogicalRelationId)); - - if (HeapTupleIsValid(oldClassTuple)) - { - CacheInvalidateRelcacheByTuple(oldClassTuple); - ReleaseSysCache(oldClassTuple); - } + CitusInvalidateRelcacheByRelid(oldLogicalRelationId); } if (newLogicalRelationId != InvalidOid) { - HeapTuple newClassTuple = - SearchSysCache1(RELOID, ObjectIdGetDatum(newLogicalRelationId)); - - if (HeapTupleIsValid(newClassTuple)) - { - CacheInvalidateRelcacheByTuple(newClassTuple); - ReleaseSysCache(newClassTuple); - } + CitusInvalidateRelcacheByRelid(newLogicalRelationId); } PG_RETURN_DATUM(PointerGetDatum(NULL)); @@ -558,26 +544,12 @@ master_dist_shard_cache_invalidate(PG_FUNCTION_ARGS) if (oldLogicalRelationId != InvalidOid && oldLogicalRelationId != newLogicalRelationId) { - HeapTuple oldClassTuple = - SearchSysCache1(RELOID, ObjectIdGetDatum(oldLogicalRelationId)); - - if (HeapTupleIsValid(oldClassTuple)) - { - CacheInvalidateRelcacheByTuple(oldClassTuple); - ReleaseSysCache(oldClassTuple); - } + CitusInvalidateRelcacheByRelid(oldLogicalRelationId); } if (newLogicalRelationId != InvalidOid) { - HeapTuple newClassTuple = - SearchSysCache1(RELOID, ObjectIdGetDatum(newLogicalRelationId)); - - if (HeapTupleIsValid(newClassTuple)) - { - CacheInvalidateRelcacheByTuple(newClassTuple); - ReleaseSysCache(newClassTuple); - } + CitusInvalidateRelcacheByRelid(newLogicalRelationId); } PG_RETURN_DATUM(PointerGetDatum(NULL)); @@ -927,3 +899,25 @@ CachedRelationLookup(const char *relationName, Oid *cachedOid) } } } + + +/* + * Register a relcache invalidation for a non-shared relation. + * + * We ignore the case that there's no corresponding pg_class entry - that + * happens if we register a relcache invalidation (e.g. for a + * pg_dist_partition deletion) after the relation has been dropped. That's ok, + * because in those cases we're guaranteed to already have registered an + * invalidation for the target relation. + */ +void +CitusInvalidateRelcacheByRelid(Oid relationId) +{ + HeapTuple classTuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relationId)); + + if (HeapTupleIsValid(classTuple)) + { + CacheInvalidateRelcacheByTuple(classTuple); + ReleaseSysCache(classTuple); + } +} diff --git a/src/include/distributed/metadata_cache.h b/src/include/distributed/metadata_cache.h index 0c002b8da..7c088832b 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -45,6 +45,7 @@ typedef struct extern bool IsDistributedTable(Oid relationId); extern ShardInterval * LoadShardInterval(uint64 shardId); extern DistTableCacheEntry * DistributedTableCacheEntry(Oid distributedRelationId); +extern void CitusInvalidateRelcacheByRelid(Oid relationId); extern bool CitusDBHasBeenLoaded(void); From 37f580f9c70af416af9b3bcc3dd3662cad32fd65 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Tue, 16 Feb 2016 14:04:12 +0100 Subject: [PATCH 3/5] Trim comment about invalidating dropped relations --- src/backend/distributed/utils/metadata_cache.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index b670f0384..ec034e2b6 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -477,9 +477,7 @@ master_dist_partition_cache_invalidate(PG_FUNCTION_ARGS) /* * Invalidate relcache for the relevant relation(s). In theory * logicalrelid should never change, but it doesn't hurt to be - * paranoid. We ignore the case that there's no corresponding pg_class - * entry - that happens if the pg_dist_partition tuple is deleted after - * the relation has been dropped. + * paranoid. */ if (oldLogicalRelationId != InvalidOid && oldLogicalRelationId != newLogicalRelationId) @@ -537,9 +535,7 @@ master_dist_shard_cache_invalidate(PG_FUNCTION_ARGS) /* * Invalidate relcache for the relevant relation(s). In theory * logicalrelid should never change, but it doesn't hurt to be - * paranoid. We ignore the case that there's no corresponding pg_class - * entry - that happens if the pg_dist_shard tuple is deleted after - * the relation has been dropped. + * paranoid. */ if (oldLogicalRelationId != InvalidOid && oldLogicalRelationId != newLogicalRelationId) From 9aa1f1e1e7d182c1fd0379d2403432e62a6462ff Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Wed, 17 Feb 2016 22:52:35 +0100 Subject: [PATCH 4/5] Rename topLevel variable to isTopLevel --- src/backend/distributed/master/master_delete_protocol.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/master/master_delete_protocol.c b/src/backend/distributed/master/master_delete_protocol.c index 9d0c6355e..b046360a6 100644 --- a/src/backend/distributed/master/master_delete_protocol.c +++ b/src/backend/distributed/master/master_delete_protocol.c @@ -96,9 +96,9 @@ master_apply_delete_command(PG_FUNCTION_ARGS) bool dontWait = false; char partitionMethod = 0; bool failOK = false; - bool topLevel = true; + bool isTopLevel = true; - PreventTransactionChain(topLevel, "master_apply_delete_command"); + PreventTransactionChain(isTopLevel, "master_apply_delete_command"); queryTreeNode = ParseTreeNode(queryString); if (!IsA(queryTreeNode, DeleteStmt)) @@ -174,11 +174,11 @@ master_drop_all_shards(PG_FUNCTION_ARGS) char *schemaName = NULL; char *relationName = NULL; - bool topLevel = true; + bool isTopLevel = true; List *shardIntervalList = NIL; int droppedShardCount = 0; - PreventTransactionChain(topLevel, "DROP distributed table"); + PreventTransactionChain(isTopLevel, "DROP distributed table"); relationName = get_rel_name(relationId); From 71af0e496e358c5fd318828049014773f9b52e0e Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Wed, 17 Feb 2016 23:33:30 +0100 Subject: [PATCH 5/5] Rename citusdb to citus in regression test output --- src/test/regress/expected/multi_table_ddl.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/regress/expected/multi_table_ddl.out b/src/test/regress/expected/multi_table_ddl.out index 0b8f0f789..e384ae32b 100644 --- a/src/test/regress/expected/multi_table_ddl.out +++ b/src/test/regress/expected/multi_table_ddl.out @@ -27,7 +27,7 @@ BEGIN; DROP TABLE testtableddl; ERROR: DROP distributed table cannot run inside a transaction block CONTEXT: SQL statement "SELECT master_drop_all_shards(v_obj.objid, v_obj.schema_name, v_obj.object_name)" -PL/pgSQL function citusdb_drop_trigger() line 15 at PERFORM +PL/pgSQL function citus_drop_trigger() line 15 at PERFORM ROLLBACK; -- verify that the table can be dropped DROP TABLE testtableddl;