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 9AB542A6C1 for ; Tue, 26 Mar 2019 13:08:58 -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 FjFMErO8hde5 for ; Tue, 26 Mar 2019 13:08:58 -0400 (EDT) Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (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 5386A28768 for ; Tue, 26 Mar 2019 13:08:58 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] Re: [PATCH v2 5/7] sql: refactor sql_trigger_step_allocate to set diag References: <607ffbbc4fcdca43ccfc5e6dabaed5ca3bc7719f.1551265819.git.kshcherbatov@tarantool.org> <403432db-e4b7-fc6f-73bb-13a98ec2959f@tarantool.org> <6fc95943-2a4b-19f3-6b66-408e6d03bd7a@tarantool.org> <26028559-c5ae-a594-b394-e4fbadf74b2b@tarantool.org> Message-ID: <342352d6-e567-9468-2905-4eb22d98b3c8@tarantool.org> Date: Tue, 26 Mar 2019 20:08:56 +0300 MIME-Version: 1.0 In-Reply-To: <26028559-c5ae-a594-b394-e4fbadf74b2b@tarantool.org> 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, Kirill Shcherbatov Thanks for the fixes! Please, look at the diff below and on the branch. ============================================================= commit 0acf8d462df1c5c095d8a1e52b90ac5c27209c34 Author: Vladislav Shpilevoy Date: Mon Mar 25 17:13:44 2019 +0300 Review fixes diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 95f489957..6c01deea4 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4113,9 +4113,10 @@ void sqlDeleteTriggerStep(sql *, TriggerStep *); * in body of a TRIGGER. * * @param db The database connection. - * @param select The SELECT statement to process. + * @param select The SELECT statement to process. Deleted on + * error. * @retval Not NULL TriggerStep object on success. - * @retval NULL Otherwise. The diag message is set. + * @retval NULL Error. The diag message is set. */ struct TriggerStep * sql_trigger_select_step(struct sql *db, struct Select *select); @@ -4127,12 +4128,13 @@ sql_trigger_select_step(struct sql *db, struct Select *select); * * @param db The database connection. * @param table_name Name of the table into which we insert. - * @param column_list List of columns in table to insert into. - * @param select The SELECT statement that supplies values. - * @param orconf The conflict algorithm - * (ON_CONFLICT_ACTION_ABORT, _REPLACE, etc.). + * @param column_list List of columns in table to insert into. Is + * deleted on error. + * @param select The SELECT statement that supplies values. Is + * deleted anyway. + * @param orconf A conflict processing algorithm. * @retval Not NULL TriggerStep object on success. - * @retval NULL Otherwise. The diag message is set. + * @retval NULL Error. The diag message is set. */ struct TriggerStep * sql_trigger_insert_step(struct sql *db, struct Token *table_name, @@ -4147,16 +4149,16 @@ sql_trigger_insert_step(struct sql *db, struct Token *table_name, * @param db The database connection. * @param table_name Name of the table to be updated. * @param new_list The SET clause: list of column and new values. - * @param where The WHERE clause. - * @param orconf The conflict algorithm - * (ON_CONFLICT_ACTION_ABORT, _REPLACE, etc.). + * Is deleted anyway. + * @param where The WHERE clause. Is deleted anyway. + * @param orconf A conflict processing algorithm. * @retval Not NULL TriggerStep object on success. - * @retval NULL Otherwise. The diag message is set. + * @retval NULL Error. The diag message is set. */ struct TriggerStep * sql_trigger_update_step(struct sql *db, struct Token *table_name, struct ExprList *new_list, struct Expr *where, - u8 orconf); + enum on_conflict_action orconf); /** * Construct a trigger step that implements a DELETE statement. @@ -4165,9 +4167,9 @@ sql_trigger_update_step(struct sql *db, struct Token *table_name, * * @param db The database connection. * @param table_name The table from which rows are deleted. - * @param where The WHERE clause. + * @param where The WHERE clause. Is deleted anyway. * @retval Not NULL TriggerStep object on success. - * @retval NULL Otherwise. The diag message is set. + * @retval NULL Error. The diag message is set. */ struct TriggerStep * sql_trigger_delete_step(struct sql *db, struct Token *table_name, diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 544ba2a5a..20838f51b 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -274,7 +274,7 @@ sql_trigger_select_step(struct sql *db, struct Select *select) * @param op Trigger opcode. * @param target_name The target name token. * @retval Not NULL TriggerStep object on success. - * @retval NULL otherwise. The diag message is set. + * @retval NULL Otherwise. The diag message is set. */ static struct TriggerStep * sql_trigger_step_new(struct sql *db, u8 op, struct Token *target_name) @@ -298,7 +298,7 @@ sql_trigger_insert_step(struct sql *db, struct Token *table_name, struct IdList *column_list, struct Select *select, enum on_conflict_action orconf) { - assert(select != 0 || db->mallocFailed); + assert(select != NULL || db->mallocFailed); struct TriggerStep *trigger_step = sql_trigger_step_new(db, TK_INSERT, table_name); if (trigger_step != NULL) { @@ -316,15 +316,14 @@ sql_trigger_insert_step(struct sql *db, struct Token *table_name, struct TriggerStep * sql_trigger_update_step(struct sql *db, struct Token *table_name, struct ExprList *new_list, struct Expr *where, - u8 orconf) + enum on_conflict_action orconf) { struct TriggerStep *trigger_step = sql_trigger_step_new(db, TK_UPDATE, table_name); if (trigger_step != NULL) { trigger_step->pExprList = sql_expr_list_dup(db, new_list, EXPRDUP_REDUCE); - trigger_step->pWhere = - sqlExprDup(db, where, EXPRDUP_REDUCE); + trigger_step->pWhere = sqlExprDup(db, where, EXPRDUP_REDUCE); trigger_step->orconf = orconf; } sql_expr_list_delete(db, new_list); @@ -339,8 +338,7 @@ sql_trigger_delete_step(struct sql *db, struct Token *table_name, struct TriggerStep *trigger_step = sql_trigger_step_new(db, TK_DELETE, table_name); if (trigger_step != NULL) { - trigger_step->pWhere = - sqlExprDup(db, where, EXPRDUP_REDUCE); + trigger_step->pWhere = sqlExprDup(db, where, EXPRDUP_REDUCE); trigger_step->orconf = ON_CONFLICT_ACTION_DEFAULT; } sql_expr_delete(db, where, false);