[Tarantool-patches] [PATCH] sql: reset values to be bound after execution

[Nikita Pettik <korablev@tarantool.org>]

Before this patch prepared statements didn't reset bound values after
its execution. As a result, if during next execution cycle not all
parameters were provided, cached values would appear. For instance:

prep = box.prepare('select :a, :b, :c')
prep:execute({{[':a'] = 1}, {[':b'] = 2}, {[':c'] = 3}}
-- [1, 2, 3]
prep:execute({{[':a'] = 1}, {[':b'] = 2}})
-- [1, 2, 3]

However, expected result for the last query should be [1, 2, NULL].
Let's fix it and always reset all binding values before next execution.

Closes #4825
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-4825-cleanup-prep-stmt-bind
Issue: https://github.com/tarantool/tarantool/issues/4825

@ChangeLog (2.3 and 2.4)
* Reset binding value placeholders after execution of prepared statement.

 src/box/execute.c          |  5 ++++
 src/box/sql/sqlInt.h       |  4 ++++
 src/box/sql/vdbeapi.c      | 17 ++++++++++++++
 test/sql/prepared.result   | 48 ++++++++++++++++++++++++++++++++++++++
 test/sql/prepared.test.lua | 10 ++++++++
 5 files changed, 84 insertions(+)

diff --git a/src/box/execute.c b/src/box/execute.c
index 24f8529ec..66eac9814 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -711,6 +711,11 @@ sql_execute_prepared(uint32_t stmt_id, const struct sql_bind *bind,
 		return sql_prepare_and_execute(sql_str, strlen(sql_str), bind,
 					       bind_count, port, region);
 	}
+	/*
+	 * Clear all set from previous execution cycle
+	 * values to be bound.
+	 */
+	sql_unbind(stmt);
 	if (sql_bind(stmt, bind, bind_count) != 0)
 		return -1;
 	enum sql_serialization_format format = sql_column_count(stmt) > 0 ?
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1579cc92e..2d6bad424 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -658,6 +658,10 @@ sql_vfs_register(sql_vfs *, int makeDflt);
 #define SQL_STMTSTATUS_AUTOINDEX         3
 #define SQL_STMTSTATUS_VM_STEP           4
 
+/** Unbind all parameters of given prepared statement. */
+void
+sql_unbind(struct sql_stmt *stmt);
+
 int
 sql_bind_blob(sql_stmt *, int, const void *,
 		  int n, void (*)(void *));
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index c984f9506..6e307e97f 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -934,6 +934,23 @@ sql_bind_type(struct Vdbe *v, uint32_t position, const char *type)
 	return 0;
 }
 
+void
+sql_unbind(struct sql_stmt *stmt)
+{
+	struct Vdbe *v = (struct Vdbe *) stmt;
+	for (int i = 1; i < v->nVar + 1; ++i) {
+		int rc = vdbeUnbind(v, i);
+		assert(rc == 0);
+		(void) rc;
+		/*
+		 * We should re-set boolean type - unassigned
+		 * binding slots are assumed to contain NULL
+		 * value, which has boolean type.
+		 */
+		sql_bind_type(v, i, "boolean");
+	}
+}
+
 /*
  * Bind a text or BLOB value.
  */
diff --git a/test/sql/prepared.result b/test/sql/prepared.result
index 5a16ea3b1..666f6e1d3 100644
--- a/test/sql/prepared.result
+++ b/test/sql/prepared.result
@@ -828,6 +828,54 @@ f2:join();
  | - true
  | ...
 
+-- gh-4825: make sure that values to be bound are erased after
+-- execution, so that they don't appear in the next statement
+-- execution.
+--
+s = prepare('SELECT :a, :b, :c');
+ | ---
+ | ...
+execute(s.stmt_id, {{[':a'] = 1}, {[':b'] = 2}, {[':c'] = 3}});
+ | ---
+ | - metadata:
+ |   - name: :a
+ |     type: integer
+ |   - name: :b
+ |     type: integer
+ |   - name: :c
+ |     type: integer
+ |   rows:
+ |   - [1, 2, 3]
+ | ...
+execute(s.stmt_id, {{[':a'] = 1}, {[':b'] = 2}});
+ | ---
+ | - metadata:
+ |   - name: :a
+ |     type: integer
+ |   - name: :b
+ |     type: integer
+ |   - name: :c
+ |     type: boolean
+ |   rows:
+ |   - [1, 2, null]
+ | ...
+execute(s.stmt_id);
+ | ---
+ | - metadata:
+ |   - name: :a
+ |     type: boolean
+ |   - name: :b
+ |     type: boolean
+ |   - name: :c
+ |     type: boolean
+ |   rows:
+ |   - [null, null, null]
+ | ...
+unprepare(s.stmt_id);
+ | ---
+ | - null
+ | ...
+
 if is_remote then
     cn:close()
     box.schema.user.revoke('guest', 'read, write, execute', 'universe')
diff --git a/test/sql/prepared.test.lua b/test/sql/prepared.test.lua
index 85116aa47..d8e8a44cb 100644
--- a/test/sql/prepared.test.lua
+++ b/test/sql/prepared.test.lua
@@ -319,6 +319,16 @@ f2:set_joinable(true)
 f1:join();
 f2:join();
 
+-- gh-4825: make sure that values to be bound are erased after
+-- execution, so that they don't appear in the next statement
+-- execution.
+--
+s = prepare('SELECT :a, :b, :c');
+execute(s.stmt_id, {{[':a'] = 1}, {[':b'] = 2}, {[':c'] = 3}});
+execute(s.stmt_id, {{[':a'] = 1}, {[':b'] = 2}});
+execute(s.stmt_id);
+unprepare(s.stmt_id);
+
 if is_remote then
     cn:close()
     box.schema.user.revoke('guest', 'read, write, execute', 'universe')
-- 
2.17.1