Tarantool development patches archive
 help / color / mirror / Atom feed
From: Ilya Kosarev <i.kosarev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: georgy@tarantool.org, i.kosarev@corp.mail.ru,
	Ilya Kosarev <i.kosarev@tarantool.org>
Subject: [tarantool-patches] [PATCH v3 5/5] refactoring: replace remaining exceptions in alter.cc & update comments
Date: Wed, 11 Sep 2019 16:05:19 +0300	[thread overview]
Message-ID: <22c8e35b32b49059714e350cc0ab91757cd816a4.1568206816.git.i.kosarev@tarantool.org> (raw)
In-Reply-To: <cover.1568206816.git.i.kosarev@tarantool.org>
In-Reply-To: <cover.1568206816.git.i.kosarev@tarantool.org>

Refactored all _xc functions in alter.cc. No exceptions left in
triggers. Now they have return codes to report errors.
trigger_run doesn't need to have try..catch anymore.
---
 src/box/alter.cc        | 348 +++++++++++++++++++++++++---------------
 src/box/ck_constraint.h |   2 +-
 src/box/schema.h        |   4 +-
 src/lib/core/trigger.cc |   9 --
 4 files changed, 222 insertions(+), 141 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index be5af4f5d..070c872fa 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -118,7 +118,7 @@ access_check_ddl(const char *name, uint32_t object_id, uint32_t owner_uid,
 }
 
 /**
- * Throw an exception if the given index definition
+ * Return -1 error code if the given index definition
  * is incompatible with a sequence.
  */
 static int
@@ -803,8 +803,15 @@ public:
 static struct trigger *
 txn_alter_trigger_new(trigger_f run, void *data)
 {
+	size_t size = sizeof(struct trigger);
 	struct trigger *trigger = (struct trigger *)
-		region_calloc_object_xc(&in_txn()->region, struct trigger);
+		region_aligned_alloc(&in_txn()->region, size,
+				     alignof(struct trigger));
+	if (trigger == NULL) {
+		diag_set(OutOfMemory, size, "region", "new slab");
+		return NULL;
+	}
+	trigger = (struct trigger *)memset(trigger, 0, size);
 	trigger->run = run;
 	trigger->data = data;
 	trigger->destroy = NULL;
@@ -982,8 +989,13 @@ alter_space_commit(struct trigger *trigger, void *event)
 	 * indexes into their new places.
 	 */
 	class AlterSpaceOp *op;
-	rlist_foreach_entry(op, &alter->ops, link)
-		op->commit(alter, signature);
+	try {
+		rlist_foreach_entry(op, &alter->ops, link) {
+			op->commit(alter, signature);
+		}
+	} catch (Exception *e) {
+		return -1;
+	}
 
 	alter->new_space = NULL; /* for alter_space_delete(). */
 	/*
@@ -1010,8 +1022,12 @@ alter_space_rollback(struct trigger *trigger, void * /* event */)
 	struct alter_space *alter = (struct alter_space *) trigger->data;
 	/* Rollback alter ops */
 	class AlterSpaceOp *op;
-	rlist_foreach_entry(op, &alter->ops, link) {
-		op->rollback(alter);
+	try {
+		rlist_foreach_entry(op, &alter->ops, link) {
+			op->rollback(alter);
+		}
+	} catch (Exception *e) {
+		return -1;
 	}
 	/* Rebuild index maps once for all indexes. */
 	space_fill_index_map(alter->old_space);
@@ -1075,6 +1091,8 @@ alter_space_do(struct txn_stmt *stmt, struct alter_space *alter)
 	struct trigger *on_commit, *on_rollback;
 	on_commit = txn_alter_trigger_new(alter_space_commit, alter);
 	on_rollback = txn_alter_trigger_new(alter_space_rollback, alter);
+	if (on_commit == NULL || on_rollback == NULL)
+		diag_raise();
 
 	/* Create a definition of the new space. */
 	space_dump_def(alter->old_space, &alter->key_list);
@@ -2030,7 +2048,9 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
 				     PRIV_C) != 0)
 			return -1;
 		RLIST_HEAD(empty_list);
-		struct space *space = space_new_xc(def, &empty_list);
+		struct space *space = space_new(def, &empty_list);
+		if (space == NULL)
+			return -1;
 		/**
 		 * The new space must be inserted in the space
 		 * cache right away to achieve linearisable
@@ -2169,9 +2189,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
 			return -1;
 		txn_stmt_on_rollback(stmt, on_rollback);
 		if (old_space->def->opts.is_view) {
-			struct Select *select =
-				sql_view_compile(sql_get(),
-						 old_space->def->opts.sql);
+			struct Select *select = sql_view_compile(sql_get(),
+								 old_space->def->opts.sql);
 			if (select == NULL)
 				return -1;
 			auto select_guard = make_scoped_guard([=] {
@@ -2253,8 +2272,11 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
 		 */
 		struct key_def **keys;
 		size_t bsize = old_space->index_count * sizeof(keys[0]);
-		keys = (struct key_def **) region_alloc_xc(&fiber()->gc,
-							   bsize);
+		keys = (struct key_def **) region_alloc(&fiber()->gc, bsize);
+		if (keys == NULL) {
+			diag_set(OutOfMemory, bsize, "region", "new slab");
+			return -1;
+		}
 		for (uint32_t i = 0; i < old_space->index_count; ++i)
 			keys[i] = old_space->index[i]->def->key_def;
 		alter->new_min_field_count =
@@ -2270,7 +2292,11 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
 		alter_space_move_indexes(alter, 0, old_space->index_id_max + 1);
 		/* Remember to update schema_version. */
 		(void) new UpdateSchemaVersion(alter);
-		alter_space_do(stmt, alter);
+		try {
+			alter_space_do(stmt, alter);
+		} catch (Exception *e) {
+			return -1;
+		}
 		alter_guard.is_active = false;
 	}
 	return 0;
@@ -2472,10 +2498,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
 		 */
 		struct key_def **keys;
 		size_t bsize = old_space->index_count * sizeof(keys[0]);
-		keys = (struct key_def **) region_alloc_xc(&fiber()->gc,
-							   bsize);
-		for (uint32_t i = 0, j = 0; i < old_space->index_count;
-		     ++i) {
+		keys = (struct key_def **) region_alloc(&fiber()->gc, bsize);
+		if (keys == NULL) {
+			diag_set(OutOfMemory, bsize, "region", "new slab");
+			return -1;
+		}
+		for (uint32_t i = 0, j = 0; i < old_space->index_count; ++i) {
 			struct index_def *d = old_space->index[i]->def;
 			if (d->iid != index_def->iid)
 				keys[j++] = d->key_def;
@@ -2528,7 +2556,11 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
 	(void) new MoveCkConstraints(alter);
 	/* Add an op to update schema_version on commit. */
 	(void) new UpdateSchemaVersion(alter);
-	alter_space_do(stmt, alter);
+	try {
+		alter_space_do(stmt, alter);
+	} catch (Exception *e) {
+		return -1;
+	}
 	scoped_guard.is_active = false;
 	return 0;
 }
@@ -2610,7 +2642,11 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
 	}
 
 	(void) new MoveCkConstraints(alter);
-	alter_space_do(stmt, alter);
+	try {
+		alter_space_do(stmt, alter);
+	} catch (Exception *e) {
+		return -1;
+	}
 	scoped_guard.is_active = false;
 	return 0;
 }
@@ -2804,7 +2840,11 @@ user_cache_alter_user(struct trigger *trigger, void * /* event */)
 		return -1;
 	auto def_guard = make_scoped_guard([=] { free(user); });
 	/* Can throw if, e.g. too many users. */
-	user_cache_replace(user);
+	try {
+		user_cache_replace(user);
+	} catch (Exception *e) {
+		return -1;
+	}
 	def_guard.is_active = false;
 	return 0;
 }
@@ -2833,7 +2873,11 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
 				     PRIV_C) != 0)
 			return -1;
 		auto def_guard = make_scoped_guard([=] { free(user); });
-		(void) user_cache_replace(user);
+		try {
+			(void) user_cache_replace(user);
+		} catch (Exception *e) {
+			return -1;
+		}
 		def_guard.is_active = false;
 		struct trigger *on_rollback =
 			txn_alter_trigger_new(user_cache_remove_user, new_tuple);
@@ -2885,7 +2929,11 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
 				     old_user->def->type, PRIV_A) != 0)
 			return -1;
 		auto def_guard = make_scoped_guard([=] { free(user); });
-		user_cache_replace(user);
+		try {
+			user_cache_replace(user);
+		} catch (Exception *e) {
+			return -1;
+		}
 		def_guard.is_active = false;
 		struct trigger *on_rollback =
 			txn_alter_trigger_new(user_cache_alter_user, old_tuple);
@@ -3133,7 +3181,8 @@ on_create_func_rollback(struct trigger *trigger, void * /* event */)
 	/* Remove the new function from the cache and delete it. */
 	struct func *func = (struct func *)trigger->data;
 	func_cache_delete(func->def->fid);
-	trigger_run_xc(&on_alter_func, func);
+	if (trigger_run(&on_alter_func, func) != 0)
+		return -1;
 	func_delete(func);
 	return 0;
 }
@@ -3153,7 +3202,8 @@ on_drop_func_rollback(struct trigger *trigger, void * /* event */)
 	/* Insert the old function back into the cache. */
 	struct func *func = (struct func *)trigger->data;
 	func_cache_insert(func);
-	trigger_run_xc(&on_alter_func, func);
+	if (trigger_run(&on_alter_func, func) != 0)
+		return -1;
 	return 0;
 }
 
@@ -3194,7 +3244,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 		func_cache_insert(func);
 		on_rollback->data = func;
 		txn_stmt_on_rollback(stmt, on_rollback);
-		trigger_run_xc(&on_alter_func, func);
+		if (trigger_run(&on_alter_func, func) != 0)
+			return -1;
 	} else if (new_tuple == NULL) {         /* DELETE */
 		uint32_t uid;
 		if (func_def_get_ids_from_tuple(old_tuple, &fid, &uid) != 0)
@@ -3227,9 +3278,10 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 		}
 		/* Can't' drop a builtin function. */
 		if (old_func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) {
-			tnt_raise(ClientError, ER_DROP_FUNCTION,
+			diag_set(ClientError, ER_DROP_FUNCTION,
 				  (unsigned) old_func->def->uid,
 				  "function is SQL built-in");
+			return -1;
 		}
 		struct trigger *on_commit =
 			txn_alter_trigger_new(on_drop_func_commit, old_func);
@@ -3240,7 +3292,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 		func_cache_delete(old_func->def->fid);
 		txn_stmt_on_commit(stmt, on_commit);
 		txn_stmt_on_rollback(stmt, on_rollback);
-		trigger_run_xc(&on_alter_func, old_func);
+		if (trigger_run(&on_alter_func, old_func) != 0)
+			return -1;
 	} else {                                /* UPDATE, REPLACE */
 		assert(new_tuple != NULL && old_tuple != NULL);
 		/**
@@ -3325,46 +3378,53 @@ coll_id_def_new_from_tuple(struct tuple *tuple, struct coll_id_def *def)
 		return -1;
 	}
 	if (base->icu.french_collation == coll_icu_on_off_MAX) {
-		tnt_raise(ClientError, ER_CANT_CREATE_COLLATION,
-			  "ICU wrong french_collation option setting, "
-				  "expected ON | OFF");
+		diag_set(ClientError, ER_CANT_CREATE_COLLATION,
+			 "ICU wrong french_collation option setting, "
+			 "expected ON | OFF");
+		return -1;
 	}
 
 	if (base->icu.alternate_handling == coll_icu_alternate_handling_MAX) {
-		tnt_raise(ClientError, ER_CANT_CREATE_COLLATION,
-			  "ICU wrong alternate_handling option setting, "
-				  "expected NON_IGNORABLE | SHIFTED");
+		diag_set(ClientError, ER_CANT_CREATE_COLLATION,
+			 "ICU wrong alternate_handling option setting, "
+			 "expected NON_IGNORABLE | SHIFTED");
+		return -1;
 	}
 
 	if (base->icu.case_first == coll_icu_case_first_MAX) {
-		tnt_raise(ClientError, ER_CANT_CREATE_COLLATION,
-			  "ICU wrong case_first option setting, "
-				  "expected OFF | UPPER_FIRST | LOWER_FIRST");
+		diag_set(ClientError, ER_CANT_CREATE_COLLATION,
+			 "ICU wrong case_first option setting, "
+			 "expected OFF | UPPER_FIRST | LOWER_FIRST");
+		return -1;
 	}
 
 	if (base->icu.case_level == coll_icu_on_off_MAX) {
-		tnt_raise(ClientError, ER_CANT_CREATE_COLLATION,
-			  "ICU wrong case_level option setting, "
-				  "expected ON | OFF");
+		diag_set(ClientError, ER_CANT_CREATE_COLLATION,
+			 "ICU wrong case_level option setting, "
+			 "expected ON | OFF");
+		return -1;
 	}
 
 	if (base->icu.normalization_mode == coll_icu_on_off_MAX) {
-		tnt_raise(ClientError, ER_CANT_CREATE_COLLATION,
-			  "ICU wrong normalization_mode option setting, "
-				  "expected ON | OFF");
+		diag_set(ClientError, ER_CANT_CREATE_COLLATION,
+			 "ICU wrong normalization_mode option setting, "
+			 "expected ON | OFF");
+		return -1;
 	}
 
 	if (base->icu.strength == coll_icu_strength_MAX) {
-		tnt_raise(ClientError, ER_CANT_CREATE_COLLATION,
-			  "ICU wrong strength option setting, "
-				  "expected PRIMARY | SECONDARY | "
-				  "TERTIARY | QUATERNARY | IDENTICAL");
+		diag_set(ClientError, ER_CANT_CREATE_COLLATION,
+			 "ICU wrong strength option setting, "
+			 "expected PRIMARY | SECONDARY | "
+			 "TERTIARY | QUATERNARY | IDENTICAL");
+		return -1;
 	}
 
 	if (base->icu.numeric_collation == coll_icu_on_off_MAX) {
-		tnt_raise(ClientError, ER_CANT_CREATE_COLLATION,
-			  "ICU wrong numeric_collation option setting, "
-				  "expected ON | OFF");
+		diag_set(ClientError, ER_CANT_CREATE_COLLATION,
+			 "ICU wrong numeric_collation option setting, "
+			 "expected ON | OFF");
+		return -1;
 	}
 	return 0;
 }
@@ -3574,60 +3634,67 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	switch (priv->object_type) {
 	case SC_UNIVERSE:
 		if (grantor->def->uid != ADMIN) {
-			tnt_raise(AccessDeniedError,
-				  priv_name(priv_type),
-				  schema_object_name(SC_UNIVERSE),
-				  name,
-				  grantor->def->name);
+			diag_set(AccessDeniedError,
+				 priv_name(priv_type),
+				 schema_object_name(SC_UNIVERSE),
+				 name,
+				 grantor->def->name);
+			return -1;
 		}
 		break;
-	case SC_SPACE:
-	{
-		struct space *space = space_cache_find_xc(priv->object_id);
+	case SC_SPACE: {
+		struct space *space = space_cache_find(priv->object_id);
+		if (space == NULL)
+			return -1;
 		if (space->def->uid != grantor->def->uid &&
 		    grantor->def->uid != ADMIN) {
-			tnt_raise(AccessDeniedError,
-				  priv_name(priv_type),
-				  schema_object_name(SC_SPACE), name,
-				  grantor->def->name);
+			diag_set(AccessDeniedError,
+				 priv_name(priv_type),
+				 schema_object_name(SC_SPACE), name,
+				 grantor->def->name);
+			return -1;
 		}
 		break;
 	}
-	case SC_FUNCTION:
-	{
+	case SC_FUNCTION: {
 		struct func *func = func_by_id(priv->object_id);
 		if (func == NULL) {
 			diag_set(ClientError, ER_NO_SUCH_FUNCTION, int2str(priv->object_id));
-			diag_raise();
+			return -1;
 		}
 		if (func->def->uid != grantor->def->uid &&
 		    grantor->def->uid != ADMIN) {
-			tnt_raise(AccessDeniedError,
-				  priv_name(priv_type),
-				  schema_object_name(SC_FUNCTION), name,
-				  grantor->def->name);
+			diag_set(AccessDeniedError,
+				 priv_name(priv_type),
+				 schema_object_name(SC_FUNCTION), name,
+				 grantor->def->name);
+			return -1;
 		}
 		break;
 	}
-	case SC_SEQUENCE:
-	{
-		struct sequence *seq = sequence_cache_find(priv->object_id);
+	case SC_SEQUENCE: {
+		struct sequence *seq = sequence_by_id(priv->object_id);
+		if (seq == NULL) {
+			diag_set(ClientError, ER_NO_SUCH_SEQUENCE, int2str(priv->object_id));
+			return -1;
+		}
 		if (seq->def->uid != grantor->def->uid &&
 		    grantor->def->uid != ADMIN) {
-			tnt_raise(AccessDeniedError,
-				  priv_name(priv_type),
-				  schema_object_name(SC_SEQUENCE), name,
-				  grantor->def->name);
+			diag_set(AccessDeniedError,
+				 priv_name(priv_type),
+				 schema_object_name(SC_SEQUENCE), name,
+				 grantor->def->name);
+			return -1;
 		}
 		break;
 	}
-	case SC_ROLE:
-	{
+	case SC_ROLE: {
 		struct user *role = user_by_id(priv->object_id);
 		if (role == NULL || role->def->type != SC_ROLE) {
-			tnt_raise(ClientError, ER_NO_SUCH_ROLE,
-				  role ? role->def->name :
-				  int2str(priv->object_id));
+			diag_set(ClientError, ER_NO_SUCH_ROLE,
+				 role ? role->def->name :
+				 int2str(priv->object_id));
+			return -1;
 		}
 		/*
 		 * Only the creator of the role can grant or revoke it.
@@ -3636,30 +3703,32 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 		if (role->def->owner != grantor->def->uid &&
 		    grantor->def->uid != ADMIN &&
 		    (role->def->uid != PUBLIC || priv->access != PRIV_X)) {
-			tnt_raise(AccessDeniedError,
-				  priv_name(priv_type),
-				  schema_object_name(SC_ROLE), name,
-				  grantor->def->name);
+			diag_set(AccessDeniedError,
+				 priv_name(priv_type),
+				 schema_object_name(SC_ROLE), name,
+				 grantor->def->name);
+			return -1;
 		}
 		/* Not necessary to do during revoke, but who cares. */
 		if (role_check(grantee, role) != 0)
-			diag_raise();
+			return -1;
 		break;
 	}
-	case SC_USER:
-	{
+	case SC_USER: {
 		struct user *user = user_by_id(priv->object_id);
 		if (user == NULL || user->def->type != SC_USER) {
-			tnt_raise(ClientError, ER_NO_SUCH_USER,
-				  user ? user->def->name :
-				  int2str(priv->object_id));
+			diag_set(ClientError, ER_NO_SUCH_USER,
+				 user ? user->def->name :
+				 int2str(priv->object_id));
+			return -1;
 		}
 		if (user->def->owner != grantor->def->uid &&
 		    grantor->def->uid != ADMIN) {
-			tnt_raise(AccessDeniedError,
-				  priv_name(priv_type),
-				  schema_object_name(SC_USER), name,
-				  grantor->def->name);
+			diag_set(AccessDeniedError,
+				 priv_name(priv_type),
+				 schema_object_name(SC_USER), name,
+				 grantor->def->name);
+			return -1;
 		}
 		break;
 	}
@@ -3667,21 +3736,22 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	case SC_ENTITY_FUNCTION:
 	case SC_ENTITY_SEQUENCE:
 	case SC_ENTITY_ROLE:
-	case SC_ENTITY_USER:
-	{
+	case SC_ENTITY_USER: {
 		/* Only admin may grant privileges on an entire entity. */
 		if (grantor->def->uid != ADMIN) {
-			tnt_raise(AccessDeniedError, priv_name(priv_type),
-				  schema_object_name(priv->object_type), name,
-				  grantor->def->name);
+			diag_set(AccessDeniedError, priv_name(priv_type),
+				 schema_object_name(priv->object_type), name,
+				 grantor->def->name);
+			return -1;
 		}
 	}
 	default:
 		break;
 	}
 	if (priv->access == 0) {
-		tnt_raise(ClientError, ER_GRANT,
-			  "the grant tuple has no privileges");
+		diag_set(ClientError, ER_GRANT,
+			 "the grant tuple has no privileges");
+		return -1;
 	}
 	return 0;
 }
@@ -4044,7 +4114,8 @@ on_create_sequence_rollback(struct trigger *trigger, void * /* event */)
 	/* Remove the new sequence from the cache and delete it. */
 	struct sequence *seq = (struct sequence *)trigger->data;
 	sequence_cache_delete(seq->def->id);
-	trigger_run_xc(&on_alter_sequence, seq);
+	if (trigger_run(&on_alter_sequence, seq) != 0)
+		return -1;
 	sequence_delete(seq);
 	return 0;
 }
@@ -4064,7 +4135,8 @@ on_drop_sequence_rollback(struct trigger *trigger, void * /* event */)
 	/* Insert the old sequence back into the cache. */
 	struct sequence *seq = (struct sequence *)trigger->data;
 	sequence_cache_insert(seq);
-	trigger_run_xc(&on_alter_sequence, seq);
+	if (trigger_run(&on_alter_sequence, seq) != 0)
+		return -1;
 	return 0;
 }
 
@@ -4087,7 +4159,8 @@ on_alter_sequence_rollback(struct trigger *trigger, void * /* event */)
 	assert(seq != NULL);
 	free(seq->def);
 	seq->def = def;
-	trigger_run_xc(&on_alter_sequence, seq);
+	if (trigger_run(&on_alter_sequence, seq) != 0)
+		return -1;
 	return 0;
 }
 
@@ -4188,7 +4261,8 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
 	}
 
 	def_guard.is_active = false;
-	trigger_run_xc(&on_alter_sequence, seq);
+	if (trigger_run(&on_alter_sequence, seq) != 0)
+		return -1;
 	return 0;
 }
 
@@ -4329,7 +4403,8 @@ set_space_sequence(struct trigger *trigger, void * /* event */)
 	space->sequence_fieldno = fieldno;
 	free(space->sequence_path);
 	space->sequence_path = path;
-	trigger_run_xc(&on_alter_space, space);
+	if (trigger_run(&on_alter_space, space) != 0)
+		return -1;
 	return 0;
 }
 
@@ -4349,7 +4424,8 @@ clear_space_sequence(struct trigger *trigger, void * /* event */)
 	space->sequence_fieldno = 0;
 	free(space->sequence_path);
 	space->sequence_path = NULL;
-	trigger_run_xc(&on_alter_space, space);
+	if (trigger_run(&on_alter_space, space) != 0)
+		return -1;
 	return 0;
 }
 
@@ -4451,7 +4527,8 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
 		space->sequence_path = NULL;
 		txn_stmt_on_rollback(stmt, on_rollback);
 	}
-	trigger_run_xc(&on_alter_space, space);
+	if (trigger_run(&on_alter_space, space) != 0)
+		return -1;
 	return 0;
 }
 
@@ -4953,16 +5030,18 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
 		if (fk_def == NULL)
 			return -1;
 		auto fk_def_guard = make_scoped_guard([=] { free(fk_def); });
-		struct space *child_space =
-			space_cache_find_xc(fk_def->child_id);
+		struct space *child_space = space_cache_find(fk_def->child_id);
+		if (child_space == NULL)
+			return -1;
 		if (child_space->def->opts.is_view) {
 			diag_set(ClientError, ER_CREATE_FK_CONSTRAINT,
-				  fk_def->name,
-				  "referencing space can't be VIEW");
+				 fk_def->name,
+				 "referencing space can't be VIEW");
 			return -1;
 		}
-		struct space *parent_space =
-			space_cache_find_xc(fk_def->parent_id);
+		struct space *parent_space = space_cache_find(fk_def->parent_id);
+		if (parent_space == NULL)
+			return -1;
 		if (parent_space->def->opts.is_view) {
 			diag_set(ClientError, ER_CREATE_FK_CONSTRAINT,
 				 fk_def->name,
@@ -5107,10 +5186,10 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
 		if (fk_def == NULL)
 			return -1;
 		auto fk_def_guard = make_scoped_guard([=] { free(fk_def); });
-		struct space *child_space =
-			space_cache_find_xc(fk_def->child_id);
-		struct space *parent_space =
-			space_cache_find_xc(fk_def->parent_id);
+		struct space *child_space = space_cache_find(fk_def->child_id);
+		struct space *parent_space = space_cache_find(fk_def->parent_id);
+		if (child_space == NULL or parent_space == NULL)
+			return -1;
 		struct fk_constraint *old_fk=
 			fk_constraint_remove(&child_space->child_fk_constraint,
 					     fk_def->name);
@@ -5184,7 +5263,8 @@ on_create_ck_constraint_rollback(struct trigger *trigger, void * /* event */)
 					   strlen(ck->def->name)) != NULL);
 	space_remove_ck_constraint(space, ck);
 	ck_constraint_delete(ck);
-	trigger_run_xc(&on_alter_space, space);
+	if (trigger_run(&on_alter_space, space) != 0)
+		return -1;
 	return 0;
 }
 
@@ -5210,7 +5290,8 @@ on_drop_ck_constraint_rollback(struct trigger *trigger, void * /* event */)
 					   strlen(ck->def->name)) == NULL);
 	if (space_add_ck_constraint(space, ck) != 0)
 		panic("Can't recover after CK constraint drop rollback");
-	trigger_run_xc(&on_alter_space, space);
+	if (trigger_run(&on_alter_space, space) != 0)
+		return -1;
 	return 0;
 }
 
@@ -5238,7 +5319,8 @@ on_replace_ck_constraint_rollback(struct trigger *trigger, void * /* event */)
 	rlist_del_entry(new_ck, link);
 	rlist_add_entry(&space->ck_constraint, ck, link);
 	ck_constraint_delete(new_ck);
-	trigger_run_xc(&on_alter_space, space);
+	if (trigger_run(&on_alter_space, space) != 0)
+		return -1;
 	return 0;
 }
 
@@ -5288,7 +5370,7 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
 		if (pk != NULL && index_size(pk) > 0) {
 			diag_set(ClientError, ER_CREATE_CK_CONSTRAINT,
 				 ck_def->name,
-				  "referencing space must be empty");
+				 "referencing space must be empty");
 			return -1;
 		}
 		struct ck_constraint *new_ck_constraint =
@@ -5338,7 +5420,9 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
 	txn_stmt_on_rollback(stmt, on_rollback);
 	txn_stmt_on_commit(stmt, on_commit);
 
-	trigger_run_xc(&on_alter_space, space);
+	if (trigger_run(&on_alter_space, space) != 0)
+		return -1;
+
 	return 0;
 }
 
@@ -5379,18 +5463,20 @@ on_replace_dd_func_index(struct trigger *trigger, void *event)
 			diag_set(ClientError, ER_NO_SUCH_FUNCTION, int2str(fid));
 			return -1;
 		}
-		func_index_check_func(func);
+		if (func_index_check_func(func) != 0)
+			return -1;
 		if (index->def->opts.func_id != func->def->fid) {
-			tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS, 0,
-				  "Function ids defined in _index and "
-				  "_func_index don't match");
+			diag_set(ClientError, ER_WRONG_INDEX_OPTIONS, 0,
+				 "Function ids defined in _index and "
+				 "_func_index don't match");
+			return -1;
 		}
 	} else if (old_tuple != NULL && new_tuple == NULL) {
 		uint32_t space_id;
 		uint32_t index_id;
 
 		if (tuple_field_u32(old_tuple,BOX_FUNC_INDEX_FIELD_SPACE_ID,
-			&space_id) != 0)
+				    &space_id) != 0)
 			return -1;
 		if (tuple_field_u32(old_tuple,BOX_FUNC_INDEX_FIELD_INDEX_ID,
 				    &index_id) != 0)
@@ -5425,7 +5511,11 @@ on_replace_dd_func_index(struct trigger *trigger, void *event)
 				 space->index_id_max + 1);
 	(void) new MoveCkConstraints(alter);
 	(void) new UpdateSchemaVersion(alter);
-	alter_space_do(stmt, alter);
+	try {
+		alter_space_do(stmt, alter);
+	} catch (Exception *e) {
+		return -1;
+	}
 
 	scoped_guard.is_active = false;
 	return 0;
diff --git a/src/box/ck_constraint.h b/src/box/ck_constraint.h
index 6af82afe6..2c27a839a 100644
--- a/src/box/ck_constraint.h
+++ b/src/box/ck_constraint.h
@@ -195,7 +195,7 @@ ck_constraint_delete(struct ck_constraint *ck_constraint);
  * pointer to make ck constraint independent of specific space
  * object version.
  *
- * Raises an exception when some ck constraint is unsatisfied.
+ * Returns error code when some ck constraint is unsatisfied.
  * The diag message is set.
  */
 int
diff --git a/src/box/schema.h b/src/box/schema.h
index 88bfd74ad..7802941ef 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -176,8 +176,8 @@ struct space *schema_space(uint32_t id);
  * Check whether or not an object has grants on it (restrict
  * constraint in drop object).
  * _priv space to look up by space id
- * @retval true object has grants
- * @retval false object has no grants
+ * @retval (bool *out) true object has grants
+ * @retval (bool *out) false object has no grants
  */
 int
 schema_find_grants(const char *type, uint32_t id, bool *out);
diff --git a/src/lib/core/trigger.cc b/src/lib/core/trigger.cc
index 9f8a477c1..7fe112f8b 100644
--- a/src/lib/core/trigger.cc
+++ b/src/lib/core/trigger.cc
@@ -30,32 +30,23 @@
  */
 
 #include "trigger.h"
-#include "exception.h"
 
 int
 trigger_run(struct rlist *list, void *event)
 {
-	try {
 	struct trigger *trigger, *tmp;
 	rlist_foreach_entry_safe(trigger, list, link, tmp)
 		if (trigger->run(trigger, event) != 0)
 			return -1;
-	} catch (Exception *e) {
-			return -1;
-	}
 	return 0;
 }
 
 int
 trigger_run_reverse(struct rlist *list, void *event)
 {
-	try {
 	struct trigger *trigger, *tmp;
 	rlist_foreach_entry_safe_reverse(trigger, list, link, tmp)
 		if (trigger->run(trigger, event) != 0)
 			return -1;
-	} catch (Exception *e) {
-			return -1;
-	}
 	return 0;
 }
-- 
2.17.1

      parent reply	other threads:[~2019-09-11 13:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-11 13:05 [tarantool-patches] [PATCH v3 0/5] refactoring: remove exceptions from triggers Ilya Kosarev
2019-09-11 13:05 ` [tarantool-patches] [PATCH v3 1/5] refactoring: remove exceptions from triggers except alter.cc Ilya Kosarev
2019-09-11 13:05 ` [tarantool-patches] [PATCH v3 2/5] refactoring: remove exceptions from used in alter.cc outer functions Ilya Kosarev
2019-09-11 13:05 ` [tarantool-patches] [PATCH v3 3/5] refactoring: replace exceptions in most alter.cc functions Ilya Kosarev
2019-09-11 13:05 ` [tarantool-patches] [PATCH v3 4/5] refactoring: remove obvious exceptions in alter.cc Ilya Kosarev
2019-09-11 13:05 ` Ilya Kosarev [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22c8e35b32b49059714e350cc0ab91757cd816a4.1568206816.git.i.kosarev@tarantool.org \
    --to=i.kosarev@tarantool.org \
    --cc=georgy@tarantool.org \
    --cc=i.kosarev@corp.mail.ru \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v3 5/5] refactoring: replace remaining exceptions in alter.cc & update comments' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox