<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hello! Thank you for review! New patch below. There won't be diff<br>
between versions as I started this version before decided to save<br>
this diff.<br>
</p>
<br>
<div class="moz-cite-prefix">On 10/03/2018 10:19 PM, Vladislav
Shpilevoy wrote:<br>
</div>
<blockquote type="cite"
cite="mid:d115efff-d3f4-184d-781e-a0912e0a4a2b@tarantool.org">Hi!
Thanks for the patch! <br>
<br>
I see that Kostja slightly reviewed it and you did not <br>
answer him, so I inlined here some of his comments to <br>
agree/disagree with them. <br>
<br>
See 8 comments below. <br>
<br>
On 28/09/2018 19:53, <a class="moz-txt-link-abbreviated"
href="mailto:imeevma@tarantool.org">imeevma@tarantool.org</a>
wrote: <br>
<blockquote type="cite">According to documentation some JDBC
functions have an ability to <br>
return all ids that were generated in executed INSERT statement.
<br>
This patch gives a way to implement such functionality. <br>
<br>
Closes #2618 <br>
--- <br>
Branch:
<a class="moz-txt-link-freetext"
href="https://github.com/tarantool/tarantool/tree/imeevma/gh-2618-return-all-generated-ids">https://github.com/tarantool/tarantool/tree/imeevma/gh-2618-return-all-generated-ids</a><br>
Issue: <a class="moz-txt-link-freetext"
href="https://github.com/tarantool/tarantool/issues/2618">https://github.com/tarantool/tarantool/issues/2618</a>
<br>
<br>
src/box/execute.c | 24 ++++++++++++++++ <br>
src/box/execute.h | 1 + <br>
src/box/lua/net_box.c | 21 ++++++++++++-- <br>
src/box/sequence.c | 29 ++++++++++++++++++++ <br>
src/box/sql/vdbe.c | 2 +- <br>
src/box/sql/vdbe.h | 3 +- <br>
src/box/sql/vdbeInt.h | 3 ++ <br>
src/box/sql/vdbeaux.c | 15 ++++++++-- <br>
src/box/txn.h | 13 +++++++++ <br>
test/sql/iproto.result | 71
++++++++++++++++++++++++++++++++++++++++++++++++ <br>
test/sql/iproto.test.lua | 15 ++++++++++ <br>
11 files changed, 190 insertions(+), 7 deletions(-) <br>
<br>
diff --git a/src/box/execute.c b/src/box/execute.c <br>
index 24459b4..d9fe33f 100644 <br>
--- a/src/box/execute.c <br>
+++ b/src/box/execute.c <br>
@@ -42,6 +42,7 @@ <br>
#include "schema.h" <br>
#include "port.h" <br>
#include "tuple.h" <br>
+#include "sql/vdbeInt.h" <br>
</blockquote>
<br>
1. Kostja said: <br>
<br>
<blockquote type="cite">
<blockquote type="cite">Ugh. You're including a header which
ends with Int. Have you <br>
thought what this Int means? It's not Integer. <br>
</blockquote>
</blockquote>
<br>
Strictly speaking, I agree with him. Src/box/execute is not <br>
an SQL internal file. It even uses SQLite public API to <br>
fetch names and types. I guess, you should implement a getter <br>
for ids list like <br>
<br>
const struct stailq * <br>
vdbe_auto_ids_list(const struct Vdbe *v); <br>
</blockquote>
Fixed.<br>
<blockquote type="cite"
cite="mid:d115efff-d3f4-184d-781e-a0912e0a4a2b@tarantool.org"> <br>
<blockquote type="cite"> const char *sql_type_strs[] = { <br>
NULL, <br>
@@ -639,14 +640,37 @@ err: <br>
assert(port_tuple->size == 0); <br>
if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) !=
0) <br>
goto err; <br>
+ struct stailq *id_list = &((struct Vdbe
*)stmt)->id_list; <br>
+ uint64_t id_count = 0; <br>
int changes = sqlite3_changes(db); <br>
int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) + <br>
mp_sizeof_uint(changes); <br>
+ if (stailq_empty(id_list) == 0) { <br>
</blockquote>
<br>
2. Kostja said: <br>
<br>
<blockquote type="cite">
<blockquote type="cite">What is the point of this == 0? Why are
you comparing with an <br>
integer here, not boolean? </blockquote>
</blockquote>
<br>
I partially agree and I guess it is my fault that I am too harsh <br>
about code style. You used here == 0 because stailq_empty() <br>
returns an integer, but its name looks like a flag getter, so you
<br>
can use it as a boolean here. <br>
</blockquote>
Fixed.<br>
<blockquote type="cite"
cite="mid:d115efff-d3f4-184d-781e-a0912e0a4a2b@tarantool.org"> <br>
<blockquote type="cite">+ struct id_entry *id_entry; <br>
+ stailq_foreach_entry(id_entry, id_list, link) { <br>
+ size += id_entry->id >= 0 ? <br>
+ mp_sizeof_uint(id_entry->id) : <br>
+ mp_sizeof_int(id_entry->id); <br>
+ id_count++; <br>
+ } <br>
+ size += mp_sizeof_uint(SQL_INFO_GENERATED_IDS) + <br>
+ mp_sizeof_array(id_count); <br>
+ }> char *buf = obuf_alloc(out, size); <br>
if (buf == NULL) { <br>
diag_set(OutOfMemory, size, "obuf_alloc", "buf");
<br>
goto err; <br>
} <br>
+ if (id_count > 0) { <br>
</blockquote>
<br>
3. Lets be consistent: either you use !stailq_empty() in both <br>
places, or id_count > 0. <br>
</blockquote>
Fixed.<br>
<blockquote type="cite"
cite="mid:d115efff-d3f4-184d-781e-a0912e0a4a2b@tarantool.org"> <br>
<blockquote type="cite">+ buf = mp_encode_uint(buf,
SQL_INFO_GENERATED_IDS); <br>
+ buf = mp_encode_array(buf, id_count); <br>
+ struct id_entry *id_entry; <br>
+ stailq_foreach_entry(id_entry, id_list, link) { <br>
+ buf = id_entry->id >= 0 ? <br>
+ mp_encode_uint(buf, id_entry->id) : <br>
+ mp_encode_int(buf, id_entry->id); <br>
+ } <br>
+ } <br>
buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT); <br>
buf = mp_encode_uint(buf, changes); <br>
} <br>
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c <br>
index a928a4c..5d2ac78 100644 <br>
--- a/src/box/lua/net_box.c <br>
+++ b/src/box/lua/net_box.c <br>
@@ -671,11 +671,28 @@ netbox_decode_sql_info(struct lua_State
*L, const char **data) <br>
/* Only SQL_INFO_ROW_COUNT is available. */ <br>
assert(map_size == 1); <br>
</blockquote>
<br>
4. How does it work? map_size can be 2 now, and the comment is <br>
outdated - not only SQL_INFO_ROW_COUNT is available. <br>
</blockquote>
Fixed.<br>
<blockquote type="cite"
cite="mid:d115efff-d3f4-184d-781e-a0912e0a4a2b@tarantool.org"> <br>
<blockquote type="cite"> (void) map_size; <br>
+ lua_newtable(L); <br>
uint32_t key = mp_decode_uint(data); <br>
+ /* <br>
+ * If SQL_INFO_GENERATED_IDS in data then it should be <br>
+ * just before SQL_INFO_ROW_COUNT. <br>
+ */ <br>
+ if (key == SQL_INFO_GENERATED_IDS) { <br>
+ uint64_t count = mp_decode_array(data); <br>
+ assert (count > 0); <br>
+ lua_createtable(L, 0, count); <br>
+ lua_setfield(L, -2, "generated_ids"); <br>
+ lua_getfield(L, -1, "generated_ids"); <br>
+ for (uint32_t j = 0; j < count; ++j) { <br>
+ int64_t value = mp_decode_uint(data); <br>
+ lua_pushinteger(L, value); <br>
+ lua_rawseti(L, -2, j + 1); <br>
+ } <br>
+ lua_pop(L, 1); <br>
+ key = mp_decode_uint(data); <br>
+ } <br>
assert(key == SQL_INFO_ROW_COUNT); <br>
- (void) key; <br>
uint32_t row_count = mp_decode_uint(data); <br>
- lua_createtable(L, 0, 1); <br>
lua_pushinteger(L, row_count); <br>
lua_setfield(L, -2, "rowcount"); <br>
} <br>
diff --git a/src/box/sequence.c b/src/box/sequence.c <br>
index 35b7605..ed6b2da 100644 <br>
--- a/src/box/sequence.c <br>
+++ b/src/box/sequence.c <br>
@@ -38,6 +38,7 @@ <br>
#include <small/mempool.h> <br>
#include <msgpuck/msgpuck.h> <br>
+#include "txn.h" <br>
#include "diag.h" <br>
#include "error.h" <br>
#include "errcode.h" <br>
@@ -178,6 +179,30 @@ sequence_update(struct sequence *seq,
int64_t value) <br>
return 0; <br>
} <br>
+/** <br>
+ * Save generated id in VDBE. <br>
+ * <br>
+ * @param value ID to save in VDBE. <br>
+ * @retval 0 Success. <br>
+ * @retval -1 Error. <br>
+ */ <br>
+static inline int <br>
+add_new_id_in_vdbe(int64_t value) <br>
</blockquote>
<br>
5. Kostja said: <br>
<br>
<blockquote type="cite">
<blockquote type="cite">subject-verb-object <br>
</blockquote>
</blockquote>
I agree. He means 'vdbe_add_new_id'. And in such a <br>
case it is evidently a part of Vdbe API, so put it into <br>
vdbe.h with this signature: <br>
<br>
int <br>
vdbe_add_new_auto_id(struct Vdbe *vdbe, int64_t id); <br>
</blockquote>
Fixed.<br>
<blockquote type="cite"
cite="mid:d115efff-d3f4-184d-781e-a0912e0a4a2b@tarantool.org"> <br>
<blockquote type="cite">+{ <br>
+ struct txn *txn = in_txn(); <br>
+ if (txn == NULL || txn->psql_txn == NULL) <br>
+ return 0; <br>
+ assert(txn->psql_txn->vdbe != NULL); <br>
+ struct id_entry *id_entry = malloc(sizeof(*id_entry)); <br>
+ if (id_entry == NULL) { <br>
+ diag_set(OutOfMemory, sizeof(*id_entry), "malloc",
"id_entry"); <br>
+ return -1; <br>
+ } <br>
+ id_entry->id = value; <br>
+ stailq_add_tail_entry(txn->psql_txn->id_list,
id_entry, link); <br>
</blockquote>
<br>
6. If you have Vdbe, you do not need to store the list both in <br>
Vdbe and sql_txn. Store it in Vdbe only. <br>
</blockquote>
Fixed.<br>
<blockquote type="cite"
cite="mid:d115efff-d3f4-184d-781e-a0912e0a4a2b@tarantool.org"> <br>
7. Kostja said: <br>
<br>
<blockquote type="cite">
<blockquote type="cite">Why malloc? <br>
</blockquote>
</blockquote>
It is understandable why malloc - txn after commit invalidates <br>
the region memory. We neither can save ids on obuf directly <br>
since while getting next id a yield is possible and the obuf <br>
will be messed with another request. But guess I thought up a <br>
solution. It is a bit tricky, so maybe it should be accounted <br>
as a separate issue and this solved with malloc. <br>
<br>
I propose to split txn_commit() into commit and region reset. <br>
Txn_commit() goal is to write data to disk, not to destroy a <br>
region. It saves us when a transaction is started and committed <br>
by Vdbe. But what about autocommit transactions? I think, we <br>
should get rid of autocommit transactions for Vdbe. Vdbe always <br>
starts a transaction for DML and commits it, but without region <br>
reset - it is a part of vdbe finalize routine (which already <br>
exists actually). <br>
<br>
This split of txn_commit logic allows to store anything on <br>
a region during Vdbe work and not being afraid about a reset. <br>
<br>
<blockquote type="cite">+ return 0; <br>
+} <br>
+ <br>
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h <br>
index d5da5d5..9b94183 100644 <br>
--- a/src/box/sql/vdbe.h <br>
+++ b/src/box/sql/vdbe.h <br>
@@ -179,10 +179,11 @@ Vdbe *sqlite3VdbeCreate(Parse *); <br>
* Allocate and initialize SQL-specific struct which completes
<br>
* original Tarantool's txn struct using region allocator. <br>
* <br>
+ * @param vdbe VDBE that is being prepared or executed. <br>
* @retval NULL on OOM, new psql_txn struct on success. <br>
**/ <br>
struct sql_txn * <br>
-sql_alloc_txn(); <br>
+sql_alloc_txn(struct Vdbe *vdbe); <br>
</blockquote>
<br>
8. I think, alloc should only alloc. sql_txn_begin should <br>
put Vdbe into sql_txn. <br>
</blockquote>
Done.<br>
<br>
<b>New patch:</b><br>
<br>
commit 9dd5e9833aec54c819e6dae07d62624538f76246<br>
Author: Mergen Imeev <a class="moz-txt-link-rfc2396E" href="mailto:imeevma@gmail.com"><imeevma@gmail.com></a><br>
Date: Wed Sep 26 22:07:54 2018 +0300<br>
<br>
sql: return all generated ids via IPROTO<br>
<br>
According to documentation some JDBC functions have an ability
to<br>
return all ids that were generated in executed INSERT statement.<br>
This patch gives a way to implement such functionality.<br>
<br>
Closes #2618<br>
<br>
diff --git a/src/box/execute.c b/src/box/execute.c<br>
index 24459b4..ebf3962 100644<br>
--- a/src/box/execute.c<br>
+++ b/src/box/execute.c<br>
@@ -42,6 +42,7 @@<br>
#include "schema.h"<br>
#include "port.h"<br>
#include "tuple.h"<br>
+#include "sql/vdbe.h"<br>
<br>
const char *sql_type_strs[] = {<br>
NULL,<br>
@@ -637,11 +638,26 @@ err:<br>
} else {<br>
keys = 1;<br>
assert(port_tuple->size == 0);<br>
- if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0)<br>
+ struct stailq *generated_ids =<br>
+ vdbe_generated_ids((struct Vdbe *)stmt);<br>
+ uint32_t map_size = stailq_empty(generated_ids) ? 1 : 2;<br>
+ if (iproto_reply_map_key(out, map_size, IPROTO_SQL_INFO) !=
0)<br>
goto err;<br>
+ uint64_t id_count = 0;<br>
int changes = sqlite3_changes(db);<br>
int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +<br>
mp_sizeof_uint(changes);<br>
+ if (!stailq_empty(generated_ids)) {<br>
+ struct id_entry *id_entry;<br>
+ stailq_foreach_entry(id_entry, generated_ids, link) {<br>
+ size += id_entry->id >= 0 ?<br>
+ mp_sizeof_uint(id_entry->id) :<br>
+ mp_sizeof_int(id_entry->id);<br>
+ id_count++;<br>
+ }<br>
+ size += mp_sizeof_uint(SQL_INFO_GENERATED_IDS) +<br>
+ mp_sizeof_array(id_count);<br>
+ }<br>
char *buf = obuf_alloc(out, size);<br>
if (buf == NULL) {<br>
diag_set(OutOfMemory, size, "obuf_alloc", "buf");<br>
@@ -649,6 +665,16 @@ err:<br>
}<br>
buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT);<br>
buf = mp_encode_uint(buf, changes);<br>
+ if (!stailq_empty(generated_ids)) {<br>
+ buf = mp_encode_uint(buf, SQL_INFO_GENERATED_IDS);<br>
+ buf = mp_encode_array(buf, id_count);<br>
+ struct id_entry *id_entry;<br>
+ stailq_foreach_entry(id_entry, generated_ids, link) {<br>
+ buf = id_entry->id >= 0 ?<br>
+ mp_encode_uint(buf, id_entry->id) :<br>
+ mp_encode_int(buf, id_entry->id);<br>
+ }<br>
+ }<br>
}<br>
iproto_reply_sql(out, &header_svp, response->sync,
schema_version,<br>
keys);<br>
diff --git a/src/box/execute.h b/src/box/execute.h<br>
index f21393b..614d3d0 100644<br>
--- a/src/box/execute.h<br>
+++ b/src/box/execute.h<br>
@@ -42,6 +42,7 @@ extern "C" {<br>
/** Keys of IPROTO_SQL_INFO map. */<br>
enum sql_info_key {<br>
SQL_INFO_ROW_COUNT = 0,<br>
+ SQL_INFO_GENERATED_IDS = 1,<br>
sql_info_key_MAX,<br>
};<br>
<br>
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c<br>
index a928a4c..1a9b3f7 100644<br>
--- a/src/box/lua/net_box.c<br>
+++ b/src/box/lua/net_box.c<br>
@@ -668,16 +668,37 @@ static void<br>
netbox_decode_sql_info(struct lua_State *L, const char **data)<br>
{<br>
uint32_t map_size = mp_decode_map(data);<br>
- /* Only SQL_INFO_ROW_COUNT is available. */<br>
- assert(map_size == 1);<br>
- (void) map_size;<br>
+ assert(map_size == 1 || map_size == 2);<br>
+ lua_newtable(L);<br>
+ /*<br>
+ * First element in data is SQL_INFO_ROW_COUNT.<br>
+ */<br>
uint32_t key = mp_decode_uint(data);<br>
assert(key == SQL_INFO_ROW_COUNT);<br>
- (void) key;<br>
uint32_t row_count = mp_decode_uint(data);<br>
- lua_createtable(L, 0, 1);<br>
lua_pushinteger(L, row_count);<br>
lua_setfield(L, -2, "rowcount");<br>
+ /*<br>
+ * If data have two elements then second is<br>
+ * SQL_INFO_GENERATED_IDS.<br>
+ */<br>
+ if (map_size == 2) {<br>
+ key = mp_decode_uint(data);<br>
+ assert(key == SQL_INFO_GENERATED_IDS);<br>
+ (void)key;<br>
+ uint64_t count = mp_decode_array(data);<br>
+ assert (count > 0);<br>
+ lua_createtable(L, 0, count);<br>
+ lua_setfield(L, -2, "generated_ids");<br>
+ lua_getfield(L, -1, "generated_ids");<br>
+ for (uint32_t j = 0; j < count; ++j) {<br>
+ int64_t id;<br>
+ mp_read_int64(data, &id);<br>
+ lua_pushinteger(L, id);<br>
+ lua_rawseti(L, -2, j + 1);<br>
+ }<br>
+ lua_pop(L, 1);<br>
+ }<br>
}<br>
<br>
static int<br>
diff --git a/src/box/sequence.c b/src/box/sequence.c<br>
index 35b7605..b1c68e9 100644<br>
--- a/src/box/sequence.c<br>
+++ b/src/box/sequence.c<br>
@@ -38,6 +38,7 @@<br>
#include <small/mempool.h><br>
#include <msgpuck/msgpuck.h><br>
<br>
+#include "txn.h"<br>
#include "diag.h"<br>
#include "error.h"<br>
#include "errcode.h"<br>
@@ -194,6 +195,9 @@ sequence_next(struct sequence *seq, int64_t
*result)<br>
new_data) == light_sequence_end)<br>
return -1;<br>
*result = def->start;<br>
+ if (txn_vdbe() != NULL &&<br>
+ vdbe_add_new_generated_id(txn_vdbe(), *result) != 0)<br>
+ return -1;<br>
return 0;<br>
}<br>
old_data = light_sequence_get(&sequence_data_index, pos);<br>
@@ -228,6 +232,9 @@ done:<br>
new_data, &old_data) == light_sequence_end)<br>
unreachable();<br>
*result = value;<br>
+ if (txn_vdbe() != NULL &&<br>
+ vdbe_add_new_generated_id(txn_vdbe(), *result) != 0)<br>
+ return -1;<br>
return 0;<br>
overflow:<br>
if (!def->cycle) {<br>
diff --git a/src/box/sequence.h b/src/box/sequence.h<br>
index 0f6c8da..d38fb2f 100644<br>
--- a/src/box/sequence.h<br>
+++ b/src/box/sequence.h<br>
@@ -43,6 +43,7 @@ extern "C" {<br>
#endif /* defined(__cplusplus) */<br>
<br>
struct iterator;<br>
+struct Vdbe;<br>
<br>
/** Sequence metadata. */<br>
struct sequence_def {<br>
@@ -140,6 +141,17 @@ int<br>
access_check_sequence(struct sequence *seq);<br>
<br>
/**<br>
+ * Add new generated id in VDBE.<br>
+ *<br>
+ * @param Vdbe VDBE to save id in.<br>
+ * @param id ID to save in VDBE.<br>
+ * @retval 0 Success.<br>
+ * @retval -1 Error.<br>
+ */<br>
+int<br>
+vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id);<br>
+<br>
+/**<br>
* Create an iterator over sequence data.<br>
*<br>
* The iterator creates a snapshot of sequence data and walks<br>
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c<br>
index 7c1015c..7a3748e 100644<br>
--- a/src/box/sql/vdbe.c<br>
+++ b/src/box/sql/vdbe.c<br>
@@ -578,6 +578,26 @@ out2Prerelease(Vdbe *p, VdbeOp *pOp)<br>
}<br>
}<br>
<br>
+struct stailq *<br>
+vdbe_generated_ids(struct Vdbe *vdbe)<br>
+{<br>
+ return &vdbe->generated_ids;<br>
+}<br>
+<br>
+int<br>
+vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id)<br>
+{<br>
+ assert(vdbe != NULL);<br>
+ struct id_entry *id_entry = region_alloc(&fiber()->gc,
sizeof(*id_entry));<br>
+ if (id_entry == NULL) {<br>
+ diag_set(OutOfMemory, sizeof(*id_entry), "region_alloc",
"id_entry");<br>
+ return -1;<br>
+ }<br>
+ id_entry->id = id;<br>
+ stailq_add_tail_entry(vdbe_generated_ids(vdbe), id_entry,
link);<br>
+ return 0;<br>
+}<br>
+<br>
/*<br>
* Execute as much of a VDBE program as we can.<br>
* This is the core of sqlite3_step().<br>
@@ -2996,7 +3016,7 @@ case OP_TransactionBegin: {<br>
*/<br>
case OP_TransactionCommit: {<br>
if (box_txn()) {<br>
- if (box_txn_commit() != 0) {<br>
+ if (txn_commit(in_txn()) != 0) {<br>
rc = SQL_TARANTOOL_ERROR;<br>
goto abort_due_to_error;<br>
}<br>
@@ -3040,7 +3060,7 @@ case OP_TransactionRollback: {<br>
*/<br>
case OP_TTransaction: {<br>
if (!box_txn()) {<br>
- if (box_txn_begin() != 0) {<br>
+ if (sql_txn_begin(p) != 0) {<br>
rc = SQL_TARANTOOL_ERROR;<br>
goto abort_due_to_error;<br>
}<br>
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h<br>
index d5da5d5..8f877b3 100644<br>
--- a/src/box/sql/vdbe.h<br>
+++ b/src/box/sql/vdbe.h<br>
@@ -197,6 +197,15 @@ sql_alloc_txn();<br>
int<br>
sql_vdbe_prepare(struct Vdbe *vdbe);<br>
<br>
+/**<br>
+ * Return pointer to list of generated ids.<br>
+ *<br>
+ * @param vdbe VDBE to get list of generated ids from.<br>
+ * @retval List of generated ids.<br>
+ */<br>
+struct stailq *<br>
+vdbe_generated_ids(struct Vdbe *vdbe);<br>
+<br>
int sqlite3VdbeAddOp0(Vdbe *, int);<br>
int sqlite3VdbeAddOp1(Vdbe *, int, int);<br>
int sqlite3VdbeAddOp2(Vdbe *, int, int, int);<br>
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h<br>
index ce97f49..37df5e2 100644<br>
--- a/src/box/sql/vdbeInt.h<br>
+++ b/src/box/sql/vdbeInt.h<br>
@@ -368,6 +368,9 @@ struct Vdbe {<br>
/** The auto-commit flag. */<br>
bool auto_commit;<br>
<br>
+ /** List of id generated in current VDBE. */<br>
+ struct stailq generated_ids;<br>
+<br>
/* When allocating a new Vdbe object, all of the fields below
should be<br>
* initialized to zero or NULL<br>
*/<br>
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c<br>
index 6e42d05..0023dc6 100644<br>
--- a/src/box/sql/vdbeaux.c<br>
+++ b/src/box/sql/vdbeaux.c<br>
@@ -57,6 +57,8 @@ sqlite3VdbeCreate(Parse * pParse)<br>
return 0;<br>
memset(p, 0, sizeof(Vdbe));<br>
p->db = db;<br>
+ /* Init list of generated ids. */<br>
+ stailq_create(&p->generated_ids);<br>
if (db->pVdbe) {<br>
db->pVdbe->pPrev = p;<br>
}<br>
@@ -106,6 +108,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe)<br>
txn->psql_txn = sql_alloc_txn();<br>
if (txn->psql_txn == NULL)<br>
return -1;<br>
+ txn->psql_txn->vdbe = vdbe;<br>
}<br>
vdbe->psql_txn = txn->psql_txn;<br>
} else {<br>
@@ -2266,6 +2269,7 @@ sql_txn_begin(Vdbe *p)<br>
box_txn_rollback();<br>
return -1;<br>
}<br>
+ ptxn->psql_txn->vdbe = p;<br>
p->psql_txn = ptxn->psql_txn;<br>
return 0;<br>
}<br>
@@ -2414,9 +2418,9 @@ sqlite3VdbeHalt(Vdbe * p)<br>
* key constraints to hold up the transaction.
This means a commit<br>
* is required.<br>
*/<br>
- rc = box_txn_commit() ==<br>
- 0 ? SQLITE_OK :<br>
- SQL_TARANTOOL_ERROR;<br>
+ rc = (in_txn() == NULL ||<br>
+ txn_commit(in_txn()) == 0) ?<br>
+ SQLITE_OK : SQL_TARANTOOL_ERROR;<br>
closeCursorsAndFree(p);<br>
}<br>
if (rc == SQLITE_BUSY && !p->pDelFrame)
{<br>
@@ -2780,6 +2784,8 @@ sqlite3VdbeDelete(Vdbe * p)<br>
}<br>
p->magic = VDBE_MAGIC_DEAD;<br>
p->db = 0;<br>
+ if (in_txn() == NULL)<br>
+ fiber_gc();<br>
sqlite3DbFree(db, p);<br>
}<br>
<br>
diff --git a/src/box/txn.h b/src/box/txn.h<br>
index e74c14d..2acba9f 100644<br>
--- a/src/box/txn.h<br>
+++ b/src/box/txn.h<br>
@@ -117,6 +117,17 @@ struct sql_txn {<br>
* VDBE to the next in the same transaction.<br>
*/<br>
uint32_t fk_deferred_count;<br>
+ /** Current VDBE. */<br>
+ struct Vdbe *vdbe;<br>
+};<br>
+<br>
+/** An element of list of generated ids. */<br>
+struct id_entry<br>
+{<br>
+ /** A link in a generated id list. */<br>
+ struct stailq_entry link;<br>
+ /** Generated id. */<br>
+ int64_t id;<br>
};<br>
<br>
struct txn {<br>
@@ -337,6 +348,20 @@ txn_last_stmt(struct txn *txn)<br>
void<br>
txn_on_stop(struct trigger *trigger, void *event);<br>
<br>
+/**<br>
+ * Return VDBE that is currently executed.<br>
+ *<br>
+ * @retval VDBE that is currently executed.<br>
+ * @retval NULL Either txn or ptxn_sql or vdbe is NULL;<br>
+ */<br>
+static inline struct Vdbe *<br>
+txn_vdbe()<br>
+{<br>
+ struct txn *txn = in_txn();<br>
+ if (txn == NULL || txn->psql_txn == NULL)<br>
+ return NULL;<br>
+ return txn->psql_txn->vdbe;<br>
+}<br>
<br>
/**<br>
* FFI bindings: do not throw exceptions, do not accept extra<br>
diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>
index af474bc..163fb6e 100644<br>
--- a/test/sql/iproto.result<br>
+++ b/test/sql/iproto.result<br>
@@ -580,6 +580,77 @@ cn:close()<br>
box.sql.execute('drop table test')<br>
---<br>
...<br>
+-- gh-2618 Return generated columns after INSERT in IPROTO.<br>
+-- Return all ids generated in current INSERT statement.<br>
+box.sql.execute('create table test (id integer primary key
autoincrement, a integer)')<br>
+---<br>
+...<br>
+cn = remote.connect(box.cfg.listen)<br>
+---<br>
+...<br>
+cn:execute('insert into test values (1, 1)')<br>
+---<br>
+- rowcount: 1<br>
+...<br>
+cn:execute('insert into test values (null, 2)')<br>
+---<br>
+- generated_ids:<br>
+ - 2<br>
+ rowcount: 1<br>
+...<br>
+cn:execute('update test set a = 11 where id == 1')<br>
+---<br>
+- rowcount: 1<br>
+...<br>
+cn:execute('insert into test values (100, 1), (null, 1), (120, 1),
(null, 1)')<br>
+---<br>
+- generated_ids:<br>
+ - 101<br>
+ - 121<br>
+ rowcount: 4<br>
+...<br>
+cn:execute('insert into test values (null, 1), (null, 1), (null,
1), (null, 1), (null, 1)')<br>
+---<br>
+- generated_ids:<br>
+ - 122<br>
+ - 123<br>
+ - 124<br>
+ - 125<br>
+ - 126<br>
+ rowcount: 5<br>
+...<br>
+cn:execute('select * from test')<br>
+---<br>
+- metadata:<br>
+ - name: ID<br>
+ - name: A<br>
+ rows:<br>
+ - [1, 11]<br>
+ - [2, 2]<br>
+ - [100, 1]<br>
+ - [101, 1]<br>
+ - [120, 1]<br>
+ - [121, 1]<br>
+ - [122, 1]<br>
+ - [123, 1]<br>
+ - [124, 1]<br>
+ - [125, 1]<br>
+ - [126, 1]<br>
+...<br>
+cn:execute('create table test2 (id integer primary key, a
integer)')<br>
+---<br>
+- rowcount: 1<br>
+...<br>
+cn:execute('drop table test2')<br>
+---<br>
+- rowcount: 1<br>
+...<br>
+cn:close()<br>
+---<br>
+...<br>
+box.sql.execute('drop table test')<br>
+---<br>
+...<br>
box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
---<br>
...<br>
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>
index 220331b..6517cde 100644<br>
--- a/test/sql/iproto.test.lua<br>
+++ b/test/sql/iproto.test.lua<br>
@@ -201,6 +201,21 @@ future4:wait_result()<br>
cn:close()<br>
box.sql.execute('drop table test')<br>
<br>
+-- gh-2618 Return generated columns after INSERT in IPROTO.<br>
+-- Return all ids generated in current INSERT statement.<br>
+box.sql.execute('create table test (id integer primary key
autoincrement, a integer)')<br>
+cn = remote.connect(box.cfg.listen)<br>
+cn:execute('insert into test values (1, 1)')<br>
+cn:execute('insert into test values (null, 2)')<br>
+cn:execute('update test set a = 11 where id == 1')<br>
+cn:execute('insert into test values (100, 1), (null, 1), (120, 1),
(null, 1)')<br>
+cn:execute('insert into test values (null, 1), (null, 1), (null,
1), (null, 1), (null, 1)')<br>
+cn:execute('select * from test')<br>
+cn:execute('create table test2 (id integer primary key, a
integer)')<br>
+cn:execute('drop table test2')<br>
+cn:close()<br>
+box.sql.execute('drop table test')<br>
+<br>
box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
space = nil<br>
<br>
<br>
<br>
</body>
</html>