Re: [Tarantool-patches] [PATCH v5 48/52] sql: introduce mem_get_bool()

[Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thank you for the review! My answer, diff and new patch below.

On Wed, Apr 14, 2021 at 01:04:46AM +0200, Vladislav Shpilevoy wrote:
> The same as for the previous getters.
Added mem_get_bool_unsafe().


Diff:


diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index cd52cb928..3d8028077 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1121,9 +1121,8 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
 			break;
 		}
 	case MP_BOOL: {
-		bool b;
-		mem_get_bool(argv[0], &b);
-		sql_result_text(context, SQL_TOKEN_BOOLEAN(b),
+		sql_result_text(context,
+				SQL_TOKEN_BOOLEAN(mem_get_bool_unsafe(argv[0])),
 				-1, SQL_TRANSIENT);
 		break;
 	}
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 4a5b9f4ea..b59197e6c 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -820,6 +820,20 @@ mem_get_double_unsafe(const struct Mem *mem)
 int
 mem_get_bool(const struct Mem *mem, bool *b);
 
+/**
+ * Return value of MEM converted to boolean. This function is not safe since
+ * there is no proper processing in case mem_get_bool() return an error. In
+ * this case this function returns FALSE.
+ */
+static inline bool
+mem_get_bool_unsafe(const struct Mem *mem)
+{
+	bool b;
+	if (mem_get_bool(mem, &b) != 0)
+		return false;
+	return b;
+}
+
 /**
  * Simple type to str convertor. It is used to simplify
  * error reporting.




New patch:


commit 64a9dfb5535f6f858a0ccf233bc1f63536591d2d
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Mar 17 14:20:37 2021 +0300

    sql: introduce mem_get_bool()
    
    This patch introduces mem_get_bool(). This function is used to receive
    boolean value from MEM. If value of MEM is not boolean, it is
    converted to boolean if possible. MEM is not changed.
    
    Part of #5818

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index dbf899c02..3d8028077 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1122,7 +1122,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
 		}
 	case MP_BOOL: {
 		sql_result_text(context,
-				SQL_TOKEN_BOOLEAN(sql_value_boolean(argv[0])),
+				SQL_TOKEN_BOOLEAN(mem_get_bool_unsafe(argv[0])),
 				-1, SQL_TRANSIENT);
 		break;
 	}
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 764735322..661764f9c 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -1124,6 +1124,16 @@ mem_get_double(const struct Mem *mem, double *d)
 	return -1;
 }
 
+int
+mem_get_bool(const struct Mem *mem, bool *b)
+{
+	if ((mem->flags & MEM_Bool) != 0) {
+		*b = mem->u.b;
+		return 0;
+	}
+	return -1;
+}
+
 int
 mem_copy(struct Mem *to, const struct Mem *from)
 {
@@ -2253,16 +2263,6 @@ releaseMemArray(Mem * p, int N)
 	}
 }
 
-int
-mem_value_bool(const struct Mem *mem, bool *b)
-{
-	if ((mem->flags  & MEM_Bool) != 0) {
-		*b = mem->u.b;
-		return 0;
-	}
-	return -1;
-}
-
 /**************************** sql_value_  ******************************
  * The following routines extract information from a Mem or sql_value
  * structure.
@@ -2289,16 +2289,6 @@ sql_value_bytes(sql_value * pVal)
 	return sqlValueBytes(pVal);
 }
 
-bool
-sql_value_boolean(sql_value *val)
-{
-	bool b = false;
-	int rc = mem_value_bool((struct Mem *) val, &b);
-	assert(rc == 0);
-	(void) rc;
-	return b;
-}
-
 const unsigned char *
 sql_value_text(sql_value * pVal)
 {
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index d283aa76a..b59197e6c 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -812,6 +812,28 @@ mem_get_double_unsafe(const struct Mem *mem)
 	return d;
 }
 
+/**
+ * Return value for MEM of BOOLEAN type. For MEM of all other types convert
+ * value of the MEM to BOOLEAN if possible and return converted value. Original
+ * MEM is not changed.
+ */
+int
+mem_get_bool(const struct Mem *mem, bool *b);
+
+/**
+ * Return value of MEM converted to boolean. This function is not safe since
+ * there is no proper processing in case mem_get_bool() return an error. In
+ * this case this function returns FALSE.
+ */
+static inline bool
+mem_get_bool_unsafe(const struct Mem *mem)
+{
+	bool b;
+	if (mem_get_bool(mem, &b) != 0)
+		return false;
+	return b;
+}
+
 /**
  * Simple type to str convertor. It is used to simplify
  * error reporting.
@@ -864,17 +886,12 @@ releaseMemArray(Mem * p, int N);
 
 /** Getters. */
 
-int
-mem_value_bool(const struct Mem *mem, bool *b);
 const void *
 sql_value_blob(struct Mem *);
 
 int
 sql_value_bytes(struct Mem *);
 
-bool
-sql_value_boolean(struct Mem *val);
-
 const unsigned char *
 sql_value_text(struct Mem *);
 
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 3f6fa1722..df9469941 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -442,9 +442,6 @@ sql_column_bytes(sql_stmt *, int iCol);
 int
 sql_column_bytes16(sql_stmt *, int iCol);
 
-bool
-sql_column_boolean(struct sql_stmt *stmt, int column);
-
 const unsigned char *
 sql_column_text(sql_stmt *,
 		    int iCol);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index daa5e0809..c75322899 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -464,12 +464,6 @@ sql_column_bytes(sql_stmt * pStmt, int i)
 	return sql_value_bytes(columnMem(pStmt, i));
 }
 
-bool
-sql_column_boolean(struct sql_stmt *stmt, int i)
-{
-	return sql_value_boolean(columnMem(stmt, i));
-}
-
 const unsigned char *
 sql_column_text(sql_stmt * pStmt, int i)
 {