[tarantool-patches] Re: [PATCH v1 12/21] sql: remove SQL_TOOBIG errcode

Mergen Imeev imeevma at tarantool.org
Sun May 26 15:12:45 MSK 2019


Hi! Thank you for review! My answer below. Also you can
there diff for patch "sql: remove SQL_RANGE errcode",
new patch "sql: remove SQL_RANGE errcode" and new patch
"sql: remove SQL_TOOBIG".


On Sat, May 25, 2019 at 07:11:00PM +0300, n.pettik wrote:
> 
> > diff --git a/src/box/bind.c b/src/box/bind.c
> > index 90d56d6..754f6c7 100644
> > --- a/src/box/bind.c
> > +++ b/src/box/bind.c
> > @@ -207,16 +207,7 @@ sql_bind_column(struct sql_stmt *stmt, const struct sql_bind *p,
> > 	}
> > 	if (rc == 0)
> > 		return 0;
> > -
> > -	switch (rc) {
> > -	case SQL_NOMEM:
> > -		diag_set(OutOfMemory, p->bytes, "vdbe", "bind value");
> > -		break;
> > -	case SQL_TOOBIG:
> > -	default:
> > -		diag_set(ClientError, ER_SQL_BIND_VALUE, sql_bind_name(p),
> > -			 mp_type_strs[p->type]);
> > -		break;
> > -	}
> > +	diag_set(ClientError, ER_SQL_BIND_VALUE, sql_bind_name(p),
> > +		 mp_type_strs[p->type]);
> 
> I guess this refactoring is incorrect a bit. Here you always set
> ER_SQL_BIND_VALUE. However, sqlVdbeMemSetStr() and
> sql_bind_zeroblob64() already set ER_SQL_EXECUTE error
> in case “string or blob is too big”. Probably, this case is not
> covered by tests. Please, come up with test case on this condition.
> 
After some research, I found that this refactoring was definitely
wrong. One reason for this is that you should not set an error
here. If one of the functions sql_bind_ *() returns not 0, it
means that the error has already been set.

After another investigation, I found that one of the assertions I
made in the "sql: remove SQL_RANGE errcode" patch shows an error.
This error is caught by pcall() in tests, so I failed to find it
earlier.

I did some refactoring in the "sql: remove SQL_RANGE errcode"
patch, which solved mentioned problem.


Diff for patch "sql: remove SQL_RANGE errcode":

diff --git a/src/box/bind.c b/src/box/bind.c
index 90d56d6..4888265 100644
--- a/src/box/bind.c
+++ b/src/box/bind.c
@@ -162,7 +162,6 @@ int
 sql_bind_column(struct sql_stmt *stmt, const struct sql_bind *p,
 		uint32_t pos)
 {
-	int rc;
 	if (p->name != NULL) {
 		pos = sql_bind_parameter_lindex(stmt, p->name, p->name_len);
 		if (pos == 0) {
@@ -174,15 +173,12 @@ sql_bind_column(struct sql_stmt *stmt, const struct sql_bind *p,
 	switch (p->type) {
 	case MP_INT:
 	case MP_UINT:
-		rc = sql_bind_int64(stmt, pos, p->i64);
-		break;
+		return sql_bind_int64(stmt, pos, p->i64);
 	case MP_BOOL:
-		rc = sql_bind_boolean(stmt, pos, p->b);
-		break;
+		return sql_bind_boolean(stmt, pos, p->b);
 	case MP_DOUBLE:
 	case MP_FLOAT:
-		rc = sql_bind_double(stmt, pos, p->d);
-		break;
+		return sql_bind_double(stmt, pos, p->d);
 	case MP_STR:
 		/*
 		 * Parameters are allocated within message pack,
@@ -192,31 +188,14 @@ sql_bind_column(struct sql_stmt *stmt, const struct sql_bind *p,
 		 * there is no need to copy the packet and we can
 		 * use SQL_STATIC.
 		 */
-		rc = sql_bind_text64(stmt, pos, p->s, p->bytes,
-					 SQL_STATIC);
-		break;
+		return sql_bind_text64(stmt, pos, p->s, p->bytes, SQL_STATIC);
 	case MP_NIL:
-		rc = sql_bind_null(stmt, pos);
-		break;
+		return sql_bind_null(stmt, pos);
 	case MP_BIN:
-		rc = sql_bind_blob64(stmt, pos, (const void *) p->s,
-					 p->bytes, SQL_STATIC);
-		break;
+		return sql_bind_blob64(stmt, pos, (const void *) p->s, p->bytes,
+				       SQL_STATIC);
 	default:
 		unreachable();
 	}
-	if (rc == 0)
-		return 0;
-
-	switch (rc) {
-	case SQL_NOMEM:
-		diag_set(OutOfMemory, p->bytes, "vdbe", "bind value");
-		break;
-	case SQL_TOOBIG:
-	default:
-		diag_set(ClientError, ER_SQL_BIND_VALUE, sql_bind_name(p),
-			 mp_type_strs[p->type]);
-		break;
-	}
-	return -1;
+	return 0;
 }
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index c697efb..49ef588 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -973,7 +973,12 @@ vdbeUnbind(Vdbe * p, int i)
 	Mem *pVar;
 	assert(p != NULL);
 	assert(p->magic == VDBE_MAGIC_RUN && p->pc < 0);
-	assert(i > 0 && i <= p->nVar);
+	assert(i > 0);
+	if(i > p->nVar) {
+		diag_set(ClientError, ER_SQL_EXECUTE, "The number of "\
+			 "parameters is too large");
+		return -1;
+	}
 	i--;
 	pVar = &p->aVar[i];
 	sqlVdbeMemRelease(pVar);
@@ -1023,19 +1028,19 @@ sql_bind_type(struct Vdbe *v, uint32_t position, const char *type)
 {
 	if (v->res_var_count < position)
 		return 0;
-	int rc = sqlVdbeSetColName(v, v->var_pos[position - 1],
-				       COLNAME_DECLTYPE, type,
-				       SQL_TRANSIENT);
+	int rc = 0;
+	if (sqlVdbeSetColName(v, v->var_pos[position - 1], COLNAME_DECLTYPE,
+			      type, SQL_TRANSIENT) != 0)
+		rc = -1;
 	const char *bind_name = v->aColName[position - 1].z;
 	if (strcmp(bind_name, "?") == 0)
 		return rc;
 	for (uint32_t i = position; i < v->res_var_count; ++i) {
 		if (strcmp(bind_name,  v->aColName[i].z) == 0) {
-			rc = sqlVdbeSetColName(v, v->var_pos[i],
-						   COLNAME_DECLTYPE, type,
-						   SQL_TRANSIENT);
-			if (rc != 0)
-				return rc;
+			if (sqlVdbeSetColName(v, v->var_pos[i],
+					      COLNAME_DECLTYPE, type,
+					      SQL_TRANSIENT) != 0)
+				return -1;
 		}
 	}
 	return 0;
@@ -1054,21 +1059,17 @@ bindText(sql_stmt * pStmt,	/* The statement to bind against */
 {
 	Vdbe *p = (Vdbe *) pStmt;
 	Mem *pVar;
-	int rc;
-
-	rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		if (zData != 0) {
-			pVar = &p->aVar[i - 1];
-			rc = sqlVdbeMemSetStr(pVar, zData, nData, 1, xDel);
-			if (rc == 0)
-				rc = sql_bind_type(p, i, "TEXT");
-			rc = sqlApiExit(p->db, rc);
-		}
-	} else if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT) {
-		xDel((void *)zData);
+	if (vdbeUnbind(p, i) != 0) {
+		if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT)
+			xDel((void *)zData);
+		return -1;
 	}
-	return rc;
+	if (zData == NULL)
+		return 0;
+	pVar = &p->aVar[i - 1];
+	if (sqlVdbeMemSetStr(pVar, zData, nData, 1, xDel) != 0)
+		return -1;
+	return sql_bind_type(p, i, "TEXT");
 }
 
 /*
@@ -1080,19 +1081,17 @@ sql_bind_blob(sql_stmt * pStmt,
     )
 {
 	struct Vdbe *p = (Vdbe *) pStmt;
-	int rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		if (zData != 0) {
-			struct Mem *var = &p->aVar[i - 1];
-			rc = sqlVdbeMemSetStr(var, zData, nData, 0, xDel);
-			if (rc == 0)
-				rc = sql_bind_type(p, i, "BLOB");
-			rc = sqlApiExit(p->db, rc);
-		}
-	} else if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT) {
-		xDel((void *)zData);
+	if (vdbeUnbind(p, i) != 0) {
+		if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT)
+			xDel((void *)zData);
+		return -1;
 	}
-	return rc;
+	if (zData == NULL)
+		return 0;
+	struct Mem *var = &p->aVar[i - 1];
+	if (sqlVdbeMemSetStr(var, zData, nData, 0, xDel) != 0)
+		return -1;
+	return sql_bind_type(p, i, "BLOB");
 }
 
 int
@@ -1113,13 +1112,11 @@ sql_bind_blob64(sql_stmt * pStmt,
 int
 sql_bind_double(sql_stmt * pStmt, int i, double rValue)
 {
-	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
-	rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		rc = sql_bind_type(p, i, "NUMERIC");
-		sqlVdbeMemSetDouble(&p->aVar[i - 1], rValue);
-	}
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	int rc = sql_bind_type(p, i, "NUMERIC");
+	sqlVdbeMemSetDouble(&p->aVar[i - 1], rValue);
 	return rc;
 }
 
@@ -1127,11 +1124,10 @@ int
 sql_bind_boolean(struct sql_stmt *stmt, int i, bool value)
 {
 	struct Vdbe *p = (struct Vdbe *) stmt;
-	int rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		rc = sql_bind_type(p, i, "BOOLEAN");
-		mem_set_bool(&p->aVar[i - 1], value);
-	}
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	int rc = sql_bind_type(p, i, "BOOLEAN");
+	mem_set_bool(&p->aVar[i - 1], value);
 	return rc;
 }
 
@@ -1144,25 +1140,21 @@ sql_bind_int(sql_stmt * p, int i, int iValue)
 int
 sql_bind_int64(sql_stmt * pStmt, int i, sql_int64 iValue)
 {
-	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
-	rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		rc = sql_bind_type(p, i, "INTEGER");
-		sqlVdbeMemSetInt64(&p->aVar[i - 1], iValue);
-	}
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	int rc = sql_bind_type(p, i, "INTEGER");
+	sqlVdbeMemSetInt64(&p->aVar[i - 1], iValue);
 	return rc;
 }
 
 int
 sql_bind_null(sql_stmt * pStmt, int i)
 {
-	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
-	rc = vdbeUnbind(p, i);
-	if (rc == 0)
-		rc = sql_bind_type(p, i, "BOOLEAN");
-	return rc;
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	return sql_bind_type(p, i, "BOOLEAN");
 }
 
 int
@@ -1191,13 +1183,11 @@ sql_bind_text64(sql_stmt * pStmt,
 int
 sql_bind_zeroblob(sql_stmt * pStmt, int i, int n)
 {
-	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
-	rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		sqlVdbeMemSetZeroBlob(&p->aVar[i - 1], n);
-	}
-	return rc;
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	sqlVdbeMemSetZeroBlob(&p->aVar[i - 1], n);
+	return 0;
 }
 
 int
diff --git a/test/sql/bind.result b/test/sql/bind.result
index 076bf83..18d546a 100644
--- a/test/sql/bind.result
+++ b/test/sql/bind.result
@@ -294,3 +294,11 @@ box.execute('DROP TABLE test')
 ---
 - row_count: 1
 ...
+box.execute('SELECT ?', {1, 2})
+---
+- error: 'Failed to execute SQL statement: The number of parameters is too large'
+...
+box.execute('SELECT $2', {1, 2, 3})
+---
+- error: 'Failed to execute SQL statement: The number of parameters is too large'
+...
diff --git a/test/sql/bind.test.lua b/test/sql/bind.test.lua
index 229207d..9dac485 100644
--- a/test/sql/bind.test.lua
+++ b/test/sql/bind.test.lua
@@ -98,3 +98,6 @@ end;
 test_run:cmd("setopt delimiter ''");
 
 box.execute('DROP TABLE test')
+
+box.execute('SELECT ?', {1, 2})
+box.execute('SELECT $2', {1, 2, 3})



New patch "sql: remove SQL_RANGE errcode":

>From 5cdc8bb97cdaa8f1da2acb8e4b4f134b85b79dd2 Mon Sep 17 00:00:00 2001
Date: Tue, 21 May 2019 17:19:28 +0300
Subject: [PATCH] sql: remove SQL_RANGE errcode

Removing this error code is part of getting rid of the SQL error
system.

diff --git a/src/box/bind.c b/src/box/bind.c
index 90d56d6..4888265 100644
--- a/src/box/bind.c
+++ b/src/box/bind.c
@@ -162,7 +162,6 @@ int
 sql_bind_column(struct sql_stmt *stmt, const struct sql_bind *p,
 		uint32_t pos)
 {
-	int rc;
 	if (p->name != NULL) {
 		pos = sql_bind_parameter_lindex(stmt, p->name, p->name_len);
 		if (pos == 0) {
@@ -174,15 +173,12 @@ sql_bind_column(struct sql_stmt *stmt, const struct sql_bind *p,
 	switch (p->type) {
 	case MP_INT:
 	case MP_UINT:
-		rc = sql_bind_int64(stmt, pos, p->i64);
-		break;
+		return sql_bind_int64(stmt, pos, p->i64);
 	case MP_BOOL:
-		rc = sql_bind_boolean(stmt, pos, p->b);
-		break;
+		return sql_bind_boolean(stmt, pos, p->b);
 	case MP_DOUBLE:
 	case MP_FLOAT:
-		rc = sql_bind_double(stmt, pos, p->d);
-		break;
+		return sql_bind_double(stmt, pos, p->d);
 	case MP_STR:
 		/*
 		 * Parameters are allocated within message pack,
@@ -192,31 +188,14 @@ sql_bind_column(struct sql_stmt *stmt, const struct sql_bind *p,
 		 * there is no need to copy the packet and we can
 		 * use SQL_STATIC.
 		 */
-		rc = sql_bind_text64(stmt, pos, p->s, p->bytes,
-					 SQL_STATIC);
-		break;
+		return sql_bind_text64(stmt, pos, p->s, p->bytes, SQL_STATIC);
 	case MP_NIL:
-		rc = sql_bind_null(stmt, pos);
-		break;
+		return sql_bind_null(stmt, pos);
 	case MP_BIN:
-		rc = sql_bind_blob64(stmt, pos, (const void *) p->s,
-					 p->bytes, SQL_STATIC);
-		break;
+		return sql_bind_blob64(stmt, pos, (const void *) p->s, p->bytes,
+				       SQL_STATIC);
 	default:
 		unreachable();
 	}
-	if (rc == 0)
-		return 0;
-
-	switch (rc) {
-	case SQL_NOMEM:
-		diag_set(OutOfMemory, p->bytes, "vdbe", "bind value");
-		break;
-	case SQL_TOOBIG:
-	default:
-		diag_set(ClientError, ER_SQL_BIND_VALUE, sql_bind_name(p),
-			 mp_type_strs[p->type]);
-		break;
-	}
-	return -1;
+	return 0;
 }
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index adf14a8..a3d99b9 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -370,8 +370,6 @@ enum sql_ret_code {
 	SQL_TOOBIG,
 	/** Abort due to constraint violation. */
 	SQL_CONSTRAINT,
-	/** 2nd parameter to sql_bind out of range. */
-	SQL_RANGE,
 	SQL_TARANTOOL_ERROR,
 	/** sql_step() has another row ready. */
 	SQL_ROW,
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index c4891a5..49ef588 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -965,11 +965,7 @@ sql_column_origin_name(sql_stmt * pStmt, int N)
  */
 /*
  * Unbind the value bound to variable i in virtual machine p. This is the
- * the same as binding a NULL value to the column. If the "i" parameter is
- * out of range, then SQL_RANGE is returned. Othewise 0.
- *
- * The error code stored in database p->db is overwritten with the return
- * value in any case.
+ * the same as binding a NULL value to the column.
  */
 static int
 vdbeUnbind(Vdbe * p, int i)
@@ -977,8 +973,11 @@ vdbeUnbind(Vdbe * p, int i)
 	Mem *pVar;
 	assert(p != NULL);
 	assert(p->magic == VDBE_MAGIC_RUN && p->pc < 0);
-	if (i < 1 || i > p->nVar) {
-		return SQL_RANGE;
+	assert(i > 0);
+	if(i > p->nVar) {
+		diag_set(ClientError, ER_SQL_EXECUTE, "The number of "\
+			 "parameters is too large");
+		return -1;
 	}
 	i--;
 	pVar = &p->aVar[i];
@@ -1029,19 +1028,19 @@ sql_bind_type(struct Vdbe *v, uint32_t position, const char *type)
 {
 	if (v->res_var_count < position)
 		return 0;
-	int rc = sqlVdbeSetColName(v, v->var_pos[position - 1],
-				       COLNAME_DECLTYPE, type,
-				       SQL_TRANSIENT);
+	int rc = 0;
+	if (sqlVdbeSetColName(v, v->var_pos[position - 1], COLNAME_DECLTYPE,
+			      type, SQL_TRANSIENT) != 0)
+		rc = -1;
 	const char *bind_name = v->aColName[position - 1].z;
 	if (strcmp(bind_name, "?") == 0)
 		return rc;
 	for (uint32_t i = position; i < v->res_var_count; ++i) {
 		if (strcmp(bind_name,  v->aColName[i].z) == 0) {
-			rc = sqlVdbeSetColName(v, v->var_pos[i],
-						   COLNAME_DECLTYPE, type,
-						   SQL_TRANSIENT);
-			if (rc != 0)
-				return rc;
+			if (sqlVdbeSetColName(v, v->var_pos[i],
+					      COLNAME_DECLTYPE, type,
+					      SQL_TRANSIENT) != 0)
+				return -1;
 		}
 	}
 	return 0;
@@ -1060,21 +1059,17 @@ bindText(sql_stmt * pStmt,	/* The statement to bind against */
 {
 	Vdbe *p = (Vdbe *) pStmt;
 	Mem *pVar;
-	int rc;
-
-	rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		if (zData != 0) {
-			pVar = &p->aVar[i - 1];
-			rc = sqlVdbeMemSetStr(pVar, zData, nData, 1, xDel);
-			if (rc == 0)
-				rc = sql_bind_type(p, i, "TEXT");
-			rc = sqlApiExit(p->db, rc);
-		}
-	} else if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT) {
-		xDel((void *)zData);
+	if (vdbeUnbind(p, i) != 0) {
+		if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT)
+			xDel((void *)zData);
+		return -1;
 	}
-	return rc;
+	if (zData == NULL)
+		return 0;
+	pVar = &p->aVar[i - 1];
+	if (sqlVdbeMemSetStr(pVar, zData, nData, 1, xDel) != 0)
+		return -1;
+	return sql_bind_type(p, i, "TEXT");
 }
 
 /*
@@ -1086,19 +1081,17 @@ sql_bind_blob(sql_stmt * pStmt,
     )
 {
 	struct Vdbe *p = (Vdbe *) pStmt;
-	int rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		if (zData != 0) {
-			struct Mem *var = &p->aVar[i - 1];
-			rc = sqlVdbeMemSetStr(var, zData, nData, 0, xDel);
-			if (rc == 0)
-				rc = sql_bind_type(p, i, "BLOB");
-			rc = sqlApiExit(p->db, rc);
-		}
-	} else if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT) {
-		xDel((void *)zData);
+	if (vdbeUnbind(p, i) != 0) {
+		if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT)
+			xDel((void *)zData);
+		return -1;
 	}
-	return rc;
+	if (zData == NULL)
+		return 0;
+	struct Mem *var = &p->aVar[i - 1];
+	if (sqlVdbeMemSetStr(var, zData, nData, 0, xDel) != 0)
+		return -1;
+	return sql_bind_type(p, i, "BLOB");
 }
 
 int
@@ -1119,13 +1112,11 @@ sql_bind_blob64(sql_stmt * pStmt,
 int
 sql_bind_double(sql_stmt * pStmt, int i, double rValue)
 {
-	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
-	rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		rc = sql_bind_type(p, i, "NUMERIC");
-		sqlVdbeMemSetDouble(&p->aVar[i - 1], rValue);
-	}
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	int rc = sql_bind_type(p, i, "NUMERIC");
+	sqlVdbeMemSetDouble(&p->aVar[i - 1], rValue);
 	return rc;
 }
 
@@ -1133,11 +1124,10 @@ int
 sql_bind_boolean(struct sql_stmt *stmt, int i, bool value)
 {
 	struct Vdbe *p = (struct Vdbe *) stmt;
-	int rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		rc = sql_bind_type(p, i, "BOOLEAN");
-		mem_set_bool(&p->aVar[i - 1], value);
-	}
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	int rc = sql_bind_type(p, i, "BOOLEAN");
+	mem_set_bool(&p->aVar[i - 1], value);
 	return rc;
 }
 
@@ -1150,25 +1140,21 @@ sql_bind_int(sql_stmt * p, int i, int iValue)
 int
 sql_bind_int64(sql_stmt * pStmt, int i, sql_int64 iValue)
 {
-	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
-	rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		rc = sql_bind_type(p, i, "INTEGER");
-		sqlVdbeMemSetInt64(&p->aVar[i - 1], iValue);
-	}
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	int rc = sql_bind_type(p, i, "INTEGER");
+	sqlVdbeMemSetInt64(&p->aVar[i - 1], iValue);
 	return rc;
 }
 
 int
 sql_bind_null(sql_stmt * pStmt, int i)
 {
-	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
-	rc = vdbeUnbind(p, i);
-	if (rc == 0)
-		rc = sql_bind_type(p, i, "BOOLEAN");
-	return rc;
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	return sql_bind_type(p, i, "BOOLEAN");
 }
 
 int
@@ -1197,13 +1183,11 @@ sql_bind_text64(sql_stmt * pStmt,
 int
 sql_bind_zeroblob(sql_stmt * pStmt, int i, int n)
 {
-	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
-	rc = vdbeUnbind(p, i);
-	if (rc == 0) {
-		sqlVdbeMemSetZeroBlob(&p->aVar[i - 1], n);
-	}
-	return rc;
+	if (vdbeUnbind(p, i) != 0)
+		return -1;
+	sqlVdbeMemSetZeroBlob(&p->aVar[i - 1], n);
+	return 0;
 }
 
 int
diff --git a/test/sql/bind.result b/test/sql/bind.result
index 076bf83..18d546a 100644
--- a/test/sql/bind.result
+++ b/test/sql/bind.result
@@ -294,3 +294,11 @@ box.execute('DROP TABLE test')
 ---
 - row_count: 1
 ...
+box.execute('SELECT ?', {1, 2})
+---
+- error: 'Failed to execute SQL statement: The number of parameters is too large'
+...
+box.execute('SELECT $2', {1, 2, 3})
+---
+- error: 'Failed to execute SQL statement: The number of parameters is too large'
+...
diff --git a/test/sql/bind.test.lua b/test/sql/bind.test.lua
index 229207d..9dac485 100644
--- a/test/sql/bind.test.lua
+++ b/test/sql/bind.test.lua
@@ -98,3 +98,6 @@ end;
 test_run:cmd("setopt delimiter ''");
 
 box.execute('DROP TABLE test')
+
+box.execute('SELECT ?', {1, 2})
+box.execute('SELECT $2', {1, 2, 3})


New patch "sql: remove SQL_TOOBIG errcode":

>From a534e6ed1b6b4411ee6f00f4d6ac252c3ad7c94a Mon Sep 17 00:00:00 2001
Date: Tue, 21 May 2019 17:44:59 +0300
Subject: [PATCH] sql: remove SQL_TOOBIG errcode

Removing this error code is part of getting rid of the SQL error
system.

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 0840a34..8e861be 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -364,8 +364,6 @@ enum sql_ret_code {
 	SQL_NOMEM,
 	/** Some kind of disk I/O error occurred. */
 	SQL_IOERR,
-	/** String or BLOB exceeds size limit. */
-	SQL_TOOBIG,
 	/** Abort due to constraint violation. */
 	SQL_CONSTRAINT,
 	SQL_TARANTOOL_ERROR,
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 1668ce0..7801fd3 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -272,7 +272,7 @@ sql_value_free(sql_value * pOld)
  *
  * The setStrOrError() function calls sqlVdbeMemSetStr() to store the
  * result as a string or blob but if the string or blob is too large, it
- * then sets the error code to SQL_TOOBIG
+ * then sets the error code.
  *
  * The invokeValueDestructor(P,X) routine invokes destructor function X()
  * on value P is not going to be used and need to be destroyed.
@@ -284,16 +284,14 @@ setResultStrOrError(sql_context * pCtx,	/* Function context */
 		    void (*xDel) (void *)	/* Destructor function */
     )
 {
-	if (sqlVdbeMemSetStr(pCtx->pOut, z, n,1, xDel) == SQL_TOOBIG) {
-		diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
+	if (sqlVdbeMemSetStr(pCtx->pOut, z, n,1, xDel) != 0)
 		pCtx->is_aborted = true;
-	}
 }
 
 static int
 invokeValueDestructor(const void *p,	/* Value to destroy */
 		      void (*xDel) (void *),	/* The destructor */
-		      sql_context * pCtx	/* Set a SQL_TOOBIG error if no NULL */
+		      sql_context * pCtx	/* Set an error if no NULL */
     )
 {
 	assert(xDel != SQL_DYNAMIC);
@@ -305,10 +303,11 @@ invokeValueDestructor(const void *p,	/* Value to destroy */
 		xDel((void *)p);
 	}
 	if (pCtx) {
-		diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
+		diag_set(ClientError, ER_SQL_EXECUTE, "string or blob is too "\
+			 "big");
 		pCtx->is_aborted = true;
 	}
-	return SQL_TOOBIG;
+	return SQL_TARANTOOL_ERROR;
 }
 
 void
@@ -317,10 +316,8 @@ sql_result_blob(sql_context * pCtx,
     )
 {
 	assert(n >= 0);
-	if (sqlVdbeMemSetStr(pCtx->pOut, z, n,0, xDel) == SQL_TOOBIG) {
-		diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
+	if (sqlVdbeMemSetStr(pCtx->pOut, z, n,0, xDel) != 0)
 		pCtx->is_aborted = true;
-	}
 }
 
 void
@@ -405,7 +402,9 @@ sql_result_zeroblob64(sql_context * pCtx, u64 n)
 {
 	Mem *pOut = pCtx->pOut;
 	if (n > (u64) pOut->db->aLimit[SQL_LIMIT_LENGTH]) {
-		return SQL_TOOBIG;
+		diag_set(ClientError, ER_SQL_EXECUTE, "string or blob is too "\
+			 "big");
+		return SQL_TARANTOOL_ERROR;
 	}
 	sqlVdbeMemSetZeroBlob(pCtx->pOut, (int)n);
 	return 0;
@@ -1179,7 +1178,9 @@ sql_bind_zeroblob64(sql_stmt * pStmt, int i, sql_uint64 n)
 	int rc;
 	Vdbe *p = (Vdbe *) pStmt;
 	if (n > (u64) p->db->aLimit[SQL_LIMIT_LENGTH]) {
-		rc = SQL_TOOBIG;
+		diag_set(ClientError, ER_SQL_EXECUTE, "string or blob is too "\
+			 "big");
+		rc = SQL_TARANTOOL_ERROR;
 	} else {
 		assert((n & 0x7FFFFFFF) == n);
 		rc = sql_bind_zeroblob(pStmt, i, n);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 4bb82b8..8be2e69 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -936,7 +936,7 @@ sqlVdbeMemMove(Mem * pTo, Mem * pFrom)
  * size limit) then no memory allocation occurs.  If the string can be
  * stored without allocating memory, then it is.  If a memory allocation
  * is required to store the string, then value of pMem is unchanged.  In
- * either case, SQL_TOOBIG is returned.
+ * either case, error is returned.
  */
 int
 sqlVdbeMemSetStr(Mem * pMem,	/* Memory cell to set to string value */
@@ -980,7 +980,9 @@ sqlVdbeMemSetStr(Mem * pMem,	/* Memory cell to set to string value */
 			nAlloc += 1; //SQL_UTF8
 		}
 		if (nByte > iLimit) {
-			return SQL_TOOBIG;
+			diag_set(ClientError, ER_SQL_EXECUTE, "string or blob "\
+				 "is too big");
+			return SQL_TARANTOOL_ERROR;
 		}
 		testcase(nAlloc == 0);
 		testcase(nAlloc == 31);
@@ -1004,7 +1006,9 @@ sqlVdbeMemSetStr(Mem * pMem,	/* Memory cell to set to string value */
 	pMem->flags = flags;
 
 	if (nByte > iLimit) {
-		return SQL_TOOBIG;
+		diag_set(ClientError, ER_SQL_EXECUTE, "string or blob is too "\
+			 "big");
+		return SQL_TARANTOOL_ERROR;
 	}
 
 	return 0;





More information about the Tarantool-patches mailing list