From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp18.mail.ru (smtp18.mail.ru [94.100.176.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 58F674696C3 for ; Fri, 3 Apr 2020 21:22:27 +0300 (MSK) From: Nikita Pettik Date: Fri, 3 Apr 2020 21:22:24 +0300 Message-Id: Subject: [Tarantool-patches] [PATCH] sql: reset values to be bound after execution List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@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