[tarantool-patches] Re: [PATCH v2 5/7] sql: refactor sql_trigger_step_allocate to set diag

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

Thanks for the fixes!

Please, look at the diff below and on the branch.

=============================================================

commit 0acf8d462df1c5c095d8a1e52b90ac5c27209c34
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
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);