[tarantool-patches] Re: [PATCH v3 2/4] collation: split collation into core and box objects

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

I added CollationError:

diff --git a/src/box/coll.c b/src/box/coll.c
index 3bf3aff3c..a8e54727b 100644
--- a/src/box/coll.c
+++ b/src/box/coll.c
@@ -46,7 +46,6 @@ box_coll_new(const struct box_coll_def *def)
  	}
  	coll->base = coll_new(&def->base);
  	if (coll->base == NULL) {
-		diag_reset(ClientError, ER_CANT_CREATE_COLLATION);
  		free(coll);
  		return NULL;
  	}
diff --git a/src/box/error.cc b/src/box/error.cc
index bbe3b5236..6b14dff05 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -108,20 +108,6 @@ ClientError::ClientError(const type_info *type, const char *file, unsigned line,
  		rmean_collect(rmean_error, RMEAN_ERROR, 1);
  }
  
-ClientError::ClientError(struct error *last_e, uint32_t errcode)
-	:Exception(&type_ClientError, last_e->file, last_e->line)
-{
-	m_errcode = errcode;
-	/*
-	 * Do not collect error - it was collected already by the
-	 * original error.
-	 */
-	int len = strlen(last_e->errmsg);
-	assert(len < DIAG_ERRMSG_MAX);
-	memcpy(this->errmsg, last_e->errmsg, len);
-	this->errmsg[len] = 0;
-}
-
  ClientError::ClientError(const char *file, unsigned line,
  			 uint32_t errcode, ...)
  	:Exception(&type_ClientError, file, line)
@@ -151,19 +137,6 @@ BuildClientError(const char *file, unsigned line, uint32_t errcode, ...)
  	}
  }
  
-struct error *
-RebuildClientError(struct error *last_e, uint32_t errcode)
-{
-	/* Can not convert OOM. */
-	if (last_e->type == &type_OutOfMemory)
-		return last_e;
-	try {
-		return new ClientError(last_e, errcode);
-	} catch (OutOfMemory *e) {
-		return e;
-	}
-}
-
  void
  ClientError::log() const
  {
@@ -182,6 +155,8 @@ ClientError::get_errcode(const struct error *e)
  		return ER_MEMORY_ISSUE;
  	if (type_cast(SystemError, e))
  		return ER_SYSTEM;
+	if (type_cast(CollationError, e))
+		return ER_CANT_CREATE_COLLATION;
  	return ER_PROC_LUA;
  }
  
diff --git a/src/box/error.h b/src/box/error.h
index 5bad1cdc3..c791e6c6a 100644
--- a/src/box/error.h
+++ b/src/box/error.h
@@ -44,9 +44,6 @@ BuildAccessDeniedError(const char *file, unsigned int line,
  		       const char *access_type, const char *object_type,
  		       const char *object_name, const char *user_name);
  
-struct error *
-RebuildClientError(struct error *last_e, uint32_t errcode);
-
  
  /** \cond public */
  
@@ -167,8 +164,6 @@ public:
  
  	ClientError(const char *file, unsigned line, uint32_t errcode, ...);
  
-	ClientError(struct error *last_e, uint32_t errcode);
-
  	static uint32_t get_errcode(const struct error *e);
  	/* client errno code */
  	int m_errcode;
diff --git a/src/coll.c b/src/coll.c
index 398bff49e..2794d5f3c 100644
--- a/src/coll.c
+++ b/src/coll.c
@@ -127,7 +127,7 @@ static int
  coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  {
  	if (def->locale_len >= MAX_LOCALE) {
-		diag_set(IllegalParams, "too long locale");
+		diag_set(CollationError, "too long locale");
  		return -1;
  	}
  	char locale[MAX_LOCALE];
@@ -136,7 +136,7 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  	UErrorCode status = U_ZERO_ERROR;
  	struct UCollator *collator = ucol_open(locale, &status);
  	if (U_FAILURE(status)) {
-		diag_set(IllegalParams, u_errorName(status));
+		diag_set(CollationError, u_errorName(status));
  		return -1;
  	}
  	coll->icu.collator = collator;
@@ -148,7 +148,7 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  				       UCOL_DEFAULT;
  		ucol_setAttribute(collator, UCOL_FRENCH_COLLATION, v, &status);
  		if (U_FAILURE(status)) {
-			diag_set(IllegalParams, tt_sprintf("failed to set "\
+			diag_set(CollationError, tt_sprintf("failed to set "\
  				 "french_collation: %s", u_errorName(status)));
  			return -1;
  		}
@@ -162,7 +162,7 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  		ucol_setAttribute(collator, UCOL_ALTERNATE_HANDLING, v,
  				  &status);
  		if (U_FAILURE(status)) {
-			diag_set(IllegalParams, tt_sprintf("failed to set "\
+			diag_set(CollationError, tt_sprintf("failed to set "\
  				 "alternate_handling: %s",
  				 u_errorName(status)));
  			return -1;
@@ -176,7 +176,7 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  			UCOL_DEFAULT;
  		ucol_setAttribute(collator, UCOL_CASE_FIRST, v, &status);
  		if (U_FAILURE(status)) {
-			diag_set(IllegalParams, tt_sprintf("failed to set "\
+			diag_set(CollationError, tt_sprintf("failed to set "\
  				 "case_first: %s", u_errorName(status)));
  			return -1;
  		}
@@ -187,7 +187,7 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  			w == COLL_ICU_OFF ? UCOL_OFF : UCOL_DEFAULT;
  		ucol_setAttribute(collator, UCOL_CASE_LEVEL , v, &status);
  		if (U_FAILURE(status)) {
-			diag_set(IllegalParams, tt_sprintf("failed to set "\
+			diag_set(CollationError, tt_sprintf("failed to set "\
  				 "case_level: %s", u_errorName(status)));
  			return -1;
  		}
@@ -199,7 +199,7 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  		ucol_setAttribute(collator, UCOL_NORMALIZATION_MODE, v,
  				  &status);
  		if (U_FAILURE(status)) {
-			diag_set(IllegalParams, tt_sprintf("failed to set "\
+			diag_set(CollationError, tt_sprintf("failed to set "\
  				 "normalization_mode: %s",
  				 u_errorName(status)));
  			return -1;
@@ -216,7 +216,7 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  			UCOL_DEFAULT;
  		ucol_setAttribute(collator, UCOL_STRENGTH, v, &status);
  		if (U_FAILURE(status)) {
-			diag_set(IllegalParams, tt_sprintf("failed to set "\
+			diag_set(CollationError, tt_sprintf("failed to set "\
  				 "strength: %s", u_errorName(status)));
  			return -1;
  		}
@@ -227,7 +227,7 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def)
  			w == COLL_ICU_OFF ? UCOL_OFF : UCOL_DEFAULT;
  		ucol_setAttribute(collator, UCOL_NUMERIC_COLLATION, v, &status);
  		if (U_FAILURE(status)) {
-			diag_set(IllegalParams, tt_sprintf("failed to set "\
+			diag_set(CollationError, tt_sprintf("failed to set "\
  				 "numeric_collation: %s", u_errorName(status)));
  			return -1;
  		}
diff --git a/src/diag.h b/src/diag.h
index 85fc1ab21..bd5a539b0 100644
--- a/src/diag.h
+++ b/src/diag.h
@@ -249,6 +249,8 @@ struct error *
  BuildSystemError(const char *file, unsigned line, const char *format, ...);
  struct error *
  BuildXlogError(const char *file, unsigned line, const char *format, ...);
+struct error *
+BuildCollationError(const char *file, unsigned line, const char *format, ...);
  
  struct index_def;
  
@@ -263,15 +265,6 @@ BuildUnsupportedIndexFeature(const char *file, unsigned line,
  	diag_add_error(diag_get(), e);					\
  } while (0)
  
-#define diag_reset(new_class, ...) do {                                 \
-	struct diag *d = diag_get();                                    \
-	struct error *last_e = diag_last_error(d);                      \
-	if (last_e->type != &type_##new_class) {                        \
-		last_e = Rebuild##new_class(last_e, ##__VA_ARGS__);     \
-		diag_add_error(d, last_e);                              \
-	}                                                               \
-} while (0)
-
  #if defined(__cplusplus)
  } /* extern "C" */
  #endif /* defined(__cplusplus) */
diff --git a/src/exception.cc b/src/exception.cc
index 56077f76d..1cbf8852f 100644
--- a/src/exception.cc
+++ b/src/exception.cc
@@ -235,6 +235,18 @@ IllegalParams::IllegalParams(const char *file, unsigned line,
  	va_end(ap);
  }
  
+const struct type_info type_CollationError =
+	make_type("CollationError", &type_Exception);
+
+CollationError::CollationError(const char *file, unsigned line,
+			       const char *format, ...)
+	: Exception(&type_CollationError, file, line)
+{
+	va_list ap;
+	va_start(ap, format);
+	error_vformat_msg(this, format, ap);
+	va_end(ap);
+}
  
  #define BuildAlloc(type)				\
  	void *p = malloc(sizeof(type));			\
@@ -303,6 +315,18 @@ BuildSystemError(const char *file, unsigned line, const char *format, ...)
  	return e;
  }
  
+struct error *
+BuildCollationError(const char *file, unsigned line, const char *format, ...)
+{
+	BuildAlloc(CollationError);
+	CollationError *e =  new (p) CollationError(file, line, "");
+	va_list ap;
+	va_start(ap, format);
+	error_vformat_msg(e, format, ap);
+	va_end(ap);
+	return e;
+}
+
  void
  exception_init()
  {
diff --git a/src/exception.h b/src/exception.h
index fe7ab84f0..f56616b68 100644
--- a/src/exception.h
+++ b/src/exception.h
@@ -49,6 +49,7 @@ extern const struct type_info type_ChannelIsClosed;
  extern const struct type_info type_LuajitError;
  extern const struct type_info type_IllegalParams;
  extern const struct type_info type_SystemError;
+extern const struct type_info type_CollationError;
  
  const char *
  exception_get_string(struct error *e, const struct method_info *method);
@@ -139,6 +140,14 @@ public:
  	IllegalParams(const char *file, unsigned line, const char *format, ...);
  	virtual void raise() { throw this; }
  };
+
+class CollationError: public Exception {
+public:
+	CollationError(const char *file, unsigned line, const char *format,
+		       ...);
+	virtual void raise() { throw this; }
+};
+
  /**
   * Initialize the exception subsystem.
   */


On 15/05/2018 22:54, Vladislav Shpilevoy wrote:
> In the issue #3290 the important problem appeared - Tarantool can
> not create completely internal collations with no ID, name,
> owner. Just for internal usage.
> 
> Original struct coll can not be used for this since
> * it has fields that are not needed in internals;
> * collation name is public thing, and the collation cache uses
>    it, so it would be necessary to forbid to a user usage of some
>    system names;
> * when multiple collations has the same comparator and only their
>    names/owners/IDs are different, the separate UCollator objects
>    are created, but it would be good to be able to reference a
>    single one.
> 
> This patch renames coll to box_coll, coll_def to box_call_def and
> introduces coll - pure collation object with no any user defined
> things.
> 
> Needed for #3290.
> ---
>   src/CMakeLists.txt       |   2 +
>   src/box/alter.cc         |  72 +++++++-------
>   src/box/coll.c           | 247 ++++-------------------------------------------
>   src/box/coll.h           |  59 +++--------
>   src/box/coll_cache.c     |  44 +++++----
>   src/box/coll_cache.h     |  17 ++--
>   src/box/coll_def.c       |  32 ------
>   src/box/coll_def.h       |  86 +----------------
>   src/box/key_def.cc       |  22 +++--
>   src/box/key_def.h        |   5 +-
>   src/box/lua/space.cc     |   8 +-
>   src/box/schema.cc        |   8 +-
>   src/box/tuple.c          |   4 +-
>   src/box/tuple_compare.cc |   5 +-
>   src/box/tuple_hash.cc    |   4 +-
>   src/coll.c               | 234 ++++++++++++++++++++++++++++++++++++++++++++
>   src/coll.h               |  98 +++++++++++++++++++
>   src/coll_def.c           |  63 ++++++++++++
>   src/coll_def.h           | 115 ++++++++++++++++++++++
>   test/unit/coll.cpp       |   8 +-
>   20 files changed, 653 insertions(+), 480 deletions(-)
>   create mode 100644 src/coll.c
>   create mode 100644 src/coll.h
>   create mode 100644 src/coll_def.c
>   create mode 100644 src/coll_def.h
>