From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id CA649212EA for ; Wed, 16 May 2018 13:07:19 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1TbwzF9ClcTb for ; Wed, 16 May 2018 13:07:19 -0400 (EDT) Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 6668D212E2 for ; Wed, 16 May 2018 13:07:19 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3 2/4] collation: split collation into core and box objects From: Vladislav Shpilevoy References: Message-ID: Date: Wed, 16 May 2018 20:07:16 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: kostja@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 >