Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] sql: reset values to be bound after execution
@ 2020-04-03 18:22 Nikita Pettik
  2020-04-03 20:07 ` Konstantin Osipov
  2020-04-03 21:23 ` Eugene Leonovich
  0 siblings, 2 replies; 7+ messages in thread
From: Nikita Pettik @ 2020-04-03 18:22 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: reset values to be bound after execution
  2020-04-03 18:22 [Tarantool-patches] [PATCH] sql: reset values to be bound after execution Nikita Pettik
@ 2020-04-03 20:07 ` Konstantin Osipov
  2020-04-06 11:20   ` Nikita Pettik
  2020-04-06 21:27   ` Vladislav Shpilevoy
  2020-04-03 21:23 ` Eugene Leonovich
  1 sibling, 2 replies; 7+ messages in thread
From: Konstantin Osipov @ 2020-04-03 20:07 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches, v.shpilevoy

* Nikita Pettik <korablev@tarantool.org> [20/04/03 21:23]:
> 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

Since this is a bug fix it perhaps should go in, but something is
wrong here with the implementation. 

How is this problem solved in sqlite?

Execution should not change vdbe in any way. Otherwise there is
a risk to end up with a complex vdbe_cleanup_after_execution()
routine (instead of throwing away execution state altogether).

> ---
> 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

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: reset values to be bound after execution
  2020-04-03 18:22 [Tarantool-patches] [PATCH] sql: reset values to be bound after execution Nikita Pettik
  2020-04-03 20:07 ` Konstantin Osipov
@ 2020-04-03 21:23 ` Eugene Leonovich
  2020-04-06 12:15   ` Nikita Pettik
  1 sibling, 1 reply; 7+ messages in thread
From: Eugene Leonovich @ 2020-04-03 21:23 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches, v.shpilevoy

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

On Fri, Apr 3, 2020 at 8:22 PM Nikita Pettik <korablev@tarantool.org> wrote:

> +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]
>

I wonder, shouldn't an error be thrown if there are not enough parameters
passed?
I just checked Postgres and MySQL, optional parameters are forbidden on
both systems:

Postgres
------------------------------------------------------------------------------------------
PREPARE foo (int, int, int) AS SELECT $1, $2, $3;
EXECUTE foo(1, 2, 3);
 ?column? | ?column? | ?column?
----------+----------+----------
        1 |        2 |        3
(1 row)

EXECUTE foo(1, 2);
ERROR:  wrong number of parameters for prepared statement "foo"
DETAIL:  Expected 3 parameters but got 2.
------------------------------------------------------------------------------------------

MySQL
------------------------------------------------------------------------------------------
PREPARE foo FROM 'SELECT ?, ?, ?';


SET @a = 1;


SET @b = 2;


SET @c = 3;
EXECUTE foo USING @a, @b, @c;
+------+------+------+
| ?    | ?    | ?    |
+------+------+------+
| 0x31 | 0x32 | 0x33 |
+------+------+------+

EXECUTE foo USING @a, @b;
ERROR 1210 (HY000): Incorrect arguments to EXECUTE
------------------------------------------------------------------------------------------


-- 
Thank you and best regards,
Eugene Leonovich

[-- Attachment #2: Type: text/html, Size: 3008 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: reset values to be bound after execution
  2020-04-03 20:07 ` Konstantin Osipov
@ 2020-04-06 11:20   ` Nikita Pettik
  2020-04-06 21:27   ` Vladislav Shpilevoy
  1 sibling, 0 replies; 7+ messages in thread
From: Nikita Pettik @ 2020-04-06 11:20 UTC (permalink / raw)
  To: Konstantin Osipov, tarantool-patches, v.shpilevoy

On 03 Apr 23:07, Konstantin Osipov wrote:
> * Nikita Pettik <korablev@tarantool.org> [20/04/03 21:23]:
> > 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
> 
> Since this is a bug fix it perhaps should go in, but something is
> wrong here with the implementation. 
> 
> How is this problem solved in sqlite?

There's no meta in sqlite ergo problem does not exist. But in SQLite
prepared statement is modified as well: for instance see
sqlite3_clear_bindings() in source code.
 
> Execution should not change vdbe in any way. Otherwise there is
> a risk to end up with a complex vdbe_cleanup_after_execution()
> routine (instead of throwing away execution state altogether).

Prepared statement is a state of VM, it is SQLite legacy. We call
sql_stmt_reset() each time after prepared statement execution. Still
it would be nice to split VM and byte-code - I already considered
this option, but it would require a lot of refactoring.

> > ---
> > 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
> 
> -- 
> Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: reset values to be bound after execution
  2020-04-03 21:23 ` Eugene Leonovich
@ 2020-04-06 12:15   ` Nikita Pettik
  0 siblings, 0 replies; 7+ messages in thread
From: Nikita Pettik @ 2020-04-06 12:15 UTC (permalink / raw)
  To: Eugene Leonovich; +Cc: tarantool-patches, v.shpilevoy

On 03 Apr 23:23, Eugene Leonovich wrote:
> On Fri, Apr 3, 2020 at 8:22 PM Nikita Pettik <korablev@tarantool.org> wrote:
> 
> > +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]
> >
> 
> I wonder, shouldn't an error be thrown if there are not enough parameters
> passed?

I guess it works just as it was implemented long ago. Some users may
rely on this 'feature'. So personally I would rather not change it.

> I just checked Postgres and MySQL, optional parameters are forbidden on
> both systems:
> 
> Postgres
> ------------------------------------------------------------------------------------------
> PREPARE foo (int, int, int) AS SELECT $1, $2, $3;
> EXECUTE foo(1, 2, 3);
>  ?column? | ?column? | ?column?
> ----------+----------+----------
>         1 |        2 |        3
> (1 row)
> 
> EXECUTE foo(1, 2);
> ERROR:  wrong number of parameters for prepared statement "foo"
> DETAIL:  Expected 3 parameters but got 2.
> ------------------------------------------------------------------------------------------
> 
> MySQL
> ------------------------------------------------------------------------------------------
> PREPARE foo FROM 'SELECT ?, ?, ?';
> 
> 
> SET @a = 1;
> 
> 
> SET @b = 2;
> 
> 
> SET @c = 3;
> EXECUTE foo USING @a, @b, @c;
> +------+------+------+
> | ?    | ?    | ?    |
> +------+------+------+
> | 0x31 | 0x32 | 0x33 |
> +------+------+------+
> 
> EXECUTE foo USING @a, @b;
> ERROR 1210 (HY000): Incorrect arguments to EXECUTE
> ------------------------------------------------------------------------------------------
> 
> 
> -- 
> Thank you and best regards,
> Eugene Leonovich

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: reset values to be bound after execution
  2020-04-03 20:07 ` Konstantin Osipov
  2020-04-06 11:20   ` Nikita Pettik
@ 2020-04-06 21:27   ` Vladislav Shpilevoy
  2020-04-07 11:40     ` Nikita Pettik
  1 sibling, 1 reply; 7+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-06 21:27 UTC (permalink / raw)
  To: Konstantin Osipov, Nikita Pettik, tarantool-patches

Hi! Thanks for the patch!

LGTM.

Kostja is probably right though. I created a ticket on
that: https://github.com/tarantool/tarantool/issues/4860

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH] sql: reset values to be bound after execution
  2020-04-06 21:27   ` Vladislav Shpilevoy
@ 2020-04-07 11:40     ` Nikita Pettik
  0 siblings, 0 replies; 7+ messages in thread
From: Nikita Pettik @ 2020-04-07 11:40 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On 06 Apr 23:27, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> LGTM.

Pushed to master and 2.3; changelogs are updated correspondingly;
branch is dropped.
 
> Kostja is probably right though. I created a ticket on
> that: https://github.com/tarantool/tarantool/issues/4860

Thx.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-04-07 11:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 18:22 [Tarantool-patches] [PATCH] sql: reset values to be bound after execution Nikita Pettik
2020-04-03 20:07 ` Konstantin Osipov
2020-04-06 11:20   ` Nikita Pettik
2020-04-06 21:27   ` Vladislav Shpilevoy
2020-04-07 11:40     ` Nikita Pettik
2020-04-03 21:23 ` Eugene Leonovich
2020-04-06 12:15   ` Nikita Pettik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox