From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 0BA5646970E for ; Fri, 27 Dec 2019 18:26:43 +0300 (MSK) References: From: Vladislav Shpilevoy Message-ID: <3efcf01a-2830-b5de-246c-e15d738612c6@tarantool.org> Date: Fri, 27 Dec 2019 18:26:42 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v5 5/5] sql: refactor PRAGMA-related code List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org Thanks for the patch! I've pushed my review fixes on top of this commit. See it below and on the branch. If you agree, then squash. Otherwise lets discuss. ================================================================================ commit 253847585514e0abc0f098c13ab77048a807d8d1 Author: Vladislav Shpilevoy Date: Fri Dec 27 18:22:53 2019 +0300 Review fixes 5/5 diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 3f3dac3be..28ba64500 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -237,15 +237,12 @@ sql_pragma_collation_list(struct Parse *parse_context) { struct Vdbe *v = sqlGetVdbe(parse_context); assert(v != NULL); - int i = 0; - struct space *space = space_by_name("_collation"); /* 16 is enough to encode 0 len array */ char key_buf[16]; - char *key_end = key_buf; - key_end = mp_encode_array(key_end, 0); + char *key_end = mp_encode_array(key_buf, 0); box_tuple_t *tuple; - box_iterator_t* it; - it = box_index_iterator(space->def->id, 0, ITER_ALL, key_buf, key_end); + box_iterator_t *it = box_index_iterator(BOX_COLLATION_ID, 0, ITER_ALL, + key_buf, key_end); if (it == NULL) { parse_context->is_aborted = true; return; @@ -253,9 +250,9 @@ sql_pragma_collation_list(struct Parse *parse_context) int rc = box_iterator_next(it, &tuple); assert(rc == 0); (void) rc; - for (i = 0; tuple != NULL; i++, box_iterator_next(it, &tuple)) { - /* 1 is name field number */ - const char *str = tuple_field_cstr(tuple, 1); + for (int i = 0; tuple != NULL; i++, box_iterator_next(it, &tuple)) { + const char *str = tuple_field_cstr(tuple, + BOX_COLLATION_FIELD_NAME); assert(str != NULL); /* this procedure should reallocate and copy str */ sqlVdbeMultiLoad(v, 1, "is", i, str); @@ -309,18 +306,18 @@ sql_pragma_foreign_key_list(struct Parse *parse_context, const char *table_name) struct fk_constraint *fk_c; rlist_foreach_entry(fk_c, &space->child_fk_constraint, in_child_space) { struct fk_constraint_def *fk_def = fk_c->def; + struct space *parent = space_by_id(fk_def->parent_id); + assert(parent != NULL); + const char *on_delete_action = + fk_constraint_action_strs[fk_def->on_delete]; + const char *on_update_action = + fk_constraint_action_strs[fk_def->on_update]; for (uint32_t j = 0; j < fk_def->field_count; j++) { - struct space *parent = space_by_id(fk_def->parent_id); - assert(parent != NULL); uint32_t ch_fl = fk_def->links[j].child_field; const char *child_col = space->def->fields[ch_fl].name; uint32_t pr_fl = fk_def->links[j].parent_field; const char *parent_col = parent->def->fields[pr_fl].name; - const char *on_delete_action = - fk_constraint_action_strs[fk_def->on_delete]; - const char *on_update_action = - fk_constraint_action_strs[fk_def->on_update]; sqlVdbeMultiLoad(v, 1, "iissssss", i, j, parent->def->name, child_col, parent_col, on_delete_action, @@ -332,21 +329,20 @@ sql_pragma_foreign_key_list(struct Parse *parse_context, const char *table_name) } void -sqlPragma(Parse *pParse, Token *pragma, Token *table, Token *index) +sqlPragma(struct Parse *pParse, struct Token *pragma, struct Token *table, + struct Token *index) { - char *pragma_name = NULL; char *table_name = NULL; char *index_name = NULL; - sql *db = pParse->db; - Vdbe *v = sqlGetVdbe(pParse); - const struct PragmaName *pPragma; + struct sql *db = pParse->db; + struct Vdbe *v = sqlGetVdbe(pParse); if (v == NULL) return; sqlVdbeRunOnlyOnce(v); pParse->nMem = 2; - pragma_name = sql_name_from_token(db, pragma); + char *pragma_name = sql_name_from_token(db, pragma); if (pragma_name == NULL) { pParse->is_aborted = true; goto pragma_out; @@ -367,7 +363,7 @@ sqlPragma(Parse *pParse, Token *pragma, Token *table, Token *index) } /* Locate the pragma in the lookup table */ - pPragma = pragmaLocate(pragma_name); + const struct PragmaName *pPragma = pragmaLocate(pragma_name); if (pPragma == 0) { diag_set(ClientError, ER_SQL_NO_SUCH_PRAGMA, pragma_name); pParse->is_aborted = true; diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h index 178746b35..f319272d4 100644 --- a/src/box/sql/pragma.h +++ b/src/box/sql/pragma.h @@ -1,17 +1,11 @@ /** List of ID of pragmas. */ enum { - /** Pragma collation_list. */ PRAGMA_COLLATION_LIST = 0, - /** Pragma foreign_key_list. */ PRAGMA_FOREIGN_KEY_LIST, - /** Pragma index_info. */ PRAGMA_INDEX_INFO, - /** Pragma index_list. */ PRAGMA_INDEX_LIST, - /** Pragma stats. */ PRAGMA_STATS, - /** Pragma table_info. */ PRAGMA_TABLE_INFO, }; diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 4691a3369..1d8f6c8cd 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -2781,7 +2781,8 @@ int sqlInit(sql *); * @param index Name of the index. */ void -sqlPragma(Parse *pParse, Token *pragma, Token *table, Token *index); +sqlPragma(struct Parse *pParse, struct Token *pragma, struct Token *table, + struct Token *index); /** * Return true if given column is part of primary key.