<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi! Thank you for review! My answer, diff between patches and new<br>
patch below. Changes were made only in commit named by "sql:<br>
return all generated ids via IPROTO". After changes were made and<br>
diff saved I rebased this branch to current 2.1. After this new<br>
patch description were formed, so it is a bit different from<br>
old patch with applied diff.<br>
<br>
<p>On 10/30/18 5:30 PM, n.pettik wrote:</p>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On 29 Oct 2018, at 20:33, <a class="moz-txt-link-abbreviated" href="mailto:imeevma@tarantool.org">imeevma@tarantool.org</a> wrote:
According to documentation some JDBC functions have an ability to
return all ids that were generated in executed INSERT statement.
This patch gives a way to implement such functionality.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">I guess you should be more specific in describing such things.
I don’t ask you to dive into details, but explaining main idea of
implementation would definitely help reviewer to do his/her job.</pre>
</blockquote>
A bit extended this message. Not sure if it is enough.<br>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Closes #2618
—
diff --git a/src/box/execute.c b/src/box/execute.c
index 24459b4..ebf3962 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -42,6 +42,7 @@
#include "schema.h"
#include "port.h"
#include "tuple.h"
+#include "sql/vdbe.h"
diff --git a/src/box/execute.h b/src/box/execute.h
index f21393b..614d3d0 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -42,6 +42,7 @@ extern "C" {
/** Keys of IPROTO_SQL_INFO map. */
enum sql_info_key {
SQL_INFO_ROW_COUNT = 0,
+ SQL_INFO_GENERATED_IDS = 1,
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">I vote for _AUTOINCREMENT_IDS, see comments below.
Anyway, in JDBC it is called GENERATED_KEYS. So, we either
follow JDBC convention or use our naming. In the latter case,
AUTOINCREMENT_IDS sounds more solid.</pre>
</blockquote>
Renamed to SQL_INFO_AUTOINCREMENT_IDS.<br>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> sql_info_key_MAX,
};
/**
+ * Append a new sequence value to Vdbe. List of automatically
+ * generated identifiers is returned as metadata of SQL response.
+ *
+ * @param Vdbe VDBE to save id in.
+ * @param id ID to save in VDBE.
+ * @retval 0 Success.
+ * @retval -1 Error.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">It looks terrible. Does this function really need comment at all?</pre>
</blockquote>
Removed.<br>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ */
+int
+vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id);
+
+int
+vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id)
+{
+ assert(vdbe != NULL);
+ struct id_entry *id_entry = region_alloc(&fiber()->gc, sizeof(*id_entry));
+ if (id_entry == NULL) {
+ diag_set(OutOfMemory, sizeof(*id_entry), "region_alloc", "id_entry”);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Out of 80.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ return -1;
+ }
+ id_entry->id = id;
+ stailq_add_tail_entry(vdbe_generated_ids(vdbe), id_entry, link);
+ return 0;
+}
+
/*
* Execute as much of a VDBE program as we can.
* This is the core of sqlite3_step().
@@ -2995,8 +3015,9 @@ case OP_TransactionBegin: {
* If there is no active transaction, raise an error.
*/
case OP_TransactionCommit: {
- if (box_txn()) {
- if (box_txn_commit() != 0) {
+ struct txn *txn = in_txn();
+ if (txn != NULL) {
+ if (txn_commit(txn) != 0) {
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">From first sigh it doesn’t look like diff related to patch.
Add comment pointing out that txn_commit() doesn’t
invoke fiber_gc() anymore and VDBE takes care of it.</pre>
</blockquote>
Extended comment of this opcode.<br>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">/**
* Prepare given VDBE to execution: initialize structs connected
@@ -197,6 +198,15 @@ sql_alloc_txn();
int
sql_vdbe_prepare(struct Vdbe *vdbe);
+/**
+ * Return pointer to list of generated ids.
+ *
+ * @param vdbe VDBE to get list of generated ids from.
+ * @retval List of generated ids.
+ */
+struct stailq *
+vdbe_generated_ids(struct Vdbe *vdbe);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">As for me, “generated_ids” seems to be too general name…
I would call it “autoincrement_ids" or sort of.</pre>
</blockquote>
Renamed to autoinc_id_list here.<br>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
int sqlite3VdbeAddOp0(Vdbe *, int);
int sqlite3VdbeAddOp1(Vdbe *, int, int);
int sqlite3VdbeAddOp2(Vdbe *, int, int, int);
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index ce97f49..7a7d3de 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -367,6 +367,11 @@ struct Vdbe {
struct sql_txn *psql_txn;
/** The auto-commit flag. */
bool auto_commit;
+ /**
+ * List of ids generated in current VDBE. It is returned
+ * as metadata of SQL response.
+ */
+ struct stailq generated_ids;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Again: “autoincrement_ids”. Term “generated_ids” can be confused
with entity ids. For example, during execution of <CREATE TABLE>
statement VDBE operates on space id, index id etc (IMHO).</pre>
</blockquote>
Renamed to autoinc_id_list here.
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> /* When allocating a new Vdbe object, all of the fields below should be
* initialized to zero or NULL
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 6e42d05..ae73f25 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -57,6 +57,7 @@ sqlite3VdbeCreate(Parse * pParse)
return 0;
memset(p, 0, sizeof(Vdbe));
p->db = db;
+ stailq_create(&p->generated_ids);
if (db->pVdbe) {
db->pVdbe->pPrev = p;
}
@@ -2414,9 +2417,9 @@ sqlite3VdbeHalt(Vdbe * p)
* key constraints to hold up the transaction. This means a commit
* is required.
*/
- rc = box_txn_commit() ==
- 0 ? SQLITE_OK :
- SQL_TARANTOOL_ERROR;
+ rc = (in_txn() == NULL ||
+ txn_commit(in_txn()) == 0) ?
+ SQLITE_OK : SQL_TARANTOOL_ERROR;
closeCursorsAndFree(p);
}
if (rc == SQLITE_BUSY && !p->pDelFrame) {
@@ -2780,6 +2783,8 @@ sqlite3VdbeDelete(Vdbe * p)
}
p->magic = VDBE_MAGIC_DEAD;
p->db = 0;
+ if (in_txn() == NULL)
+ fiber_gc();
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Describe this change (in code, obviously):
“VDBE is responsible for releasing region …"
and so on.</pre>
</blockquote>
Added comment.<br>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> sqlite3DbFree(db, p);
}
diff --git a/src/box/txn.h b/src/box/txn.h
index e74c14d..87b55da 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -49,6 +49,7 @@ struct engine;
struct space;
struct tuple;
struct xrow_header;
+struct Vdbe;
enum {
/**
@@ -117,6 +118,19 @@ struct sql_txn {
* VDBE to the next in the same transaction.
*/
uint32_t fk_deferred_count;
+ /** Current VDBE. */
+ struct Vdbe *vdbe;
+};
+
+/**
+ * An element of list of autogenerated ids, being returned as SQL
+ * response metadata.
+ */
+struct id_entry {
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">“id_entry” is too common name, be more specific pls. </pre>
</blockquote>
Renamed to struct autoinc_id_entry. Still, name of variable is<br>
'id_entry'.<br>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ /** A link in a generated id list. */
+ struct stailq_entry link;
+ /** Generated id. */
+ int64_t id;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">These comments are too obvious, they only contaminate
source code. </pre>
</blockquote>
Removed these comments.<br>
<blockquote type="cite"
cite="mid:6167E418-CCCC-4596-9E6C-F04C02045456@tarantool.org">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">};
/**
* FFI bindings: do not throw exceptions, do not accept extra
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index af474bc..af4fc12 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -580,6 +580,98 @@ cn:close()
box.sql.execute('drop table test')
---
...
+-- gh-2618 Return generated columns after INSERT in IPROTO.
+-- Return all ids generated in current INSERT statement.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Add pls case where generated ids are less than 0.</pre>
</blockquote>
<p>Added.</p>
<p><br>
<b>Diff between versions:</b></p>
<p>diff --git a/src/box/execute.c b/src/box/execute.c<br>
index ebf3962..64c0be6 100644<br>
--- a/src/box/execute.c<br>
+++ b/src/box/execute.c<br>
@@ -638,24 +638,24 @@ err:<br>
} else {<br>
keys = 1;<br>
assert(port_tuple->size == 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>
+ struct stailq *autoinc_id_list =<br>
+ vdbe_autoinc_id_list((struct Vdbe *)stmt);<br>
+ uint32_t map_size = stailq_empty(autoinc_id_list) ? 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>
+ if (!stailq_empty(autoinc_id_list)) {<br>
+ struct autoinc_id_entry *id_entry;<br>
+ stailq_foreach_entry(id_entry, autoinc_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>
+ size += mp_sizeof_uint(SQL_INFO_AUTOINCREMENT_IDS) +<br>
mp_sizeof_array(id_count);<br>
}<br>
char *buf = obuf_alloc(out, size);<br>
@@ -665,11 +665,11 @@ 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>
+ if (!stailq_empty(autoinc_id_list)) {<br>
+ buf = mp_encode_uint(buf, SQL_INFO_AUTOINCREMENT_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>
+ struct autoinc_id_entry *id_entry;<br>
+ stailq_foreach_entry(id_entry, autoinc_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>
diff --git a/src/box/execute.h b/src/box/execute.h<br>
index 614d3d0..77bfd79 100644<br>
--- a/src/box/execute.h<br>
+++ b/src/box/execute.h<br>
@@ -42,7 +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_AUTOINCREMENT_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 5a137e0..836ed6e 100644<br>
--- a/src/box/lua/net_box.c<br>
+++ b/src/box/lua/net_box.c<br>
@@ -680,11 +680,11 @@ netbox_decode_sql_info(struct lua_State *L,
const char **data)<br>
lua_setfield(L, -2, "rowcount");<br>
/*<br>
* If data have two elements then second is<br>
- * SQL_INFO_GENERATED_IDS.<br>
+ * SQL_INFO_AUTOINCREMENT_IDS.<br>
*/<br>
if (map_size == 2) {<br>
key = mp_decode_uint(data);<br>
- assert(key == SQL_INFO_GENERATED_IDS);<br>
+ assert(key == SQL_INFO_AUTOINCREMENT_IDS);<br>
(void)key;<br>
uint64_t count = mp_decode_array(data);<br>
assert(count > 0);<br>
@@ -695,7 +695,7 @@ netbox_decode_sql_info(struct lua_State *L,
const char **data)<br>
luaL_pushint64(L, id);<br>
lua_rawseti(L, -2, j + 1);<br>
}<br>
- lua_setfield(L, -2, "generated_ids");<br>
+ lua_setfield(L, -2, "autoincrement_ids");<br>
}<br>
}<br>
<br>
diff --git a/src/box/sequence.c b/src/box/sequence.c<br>
index 9a87a56..c9828c0 100644<br>
--- a/src/box/sequence.c<br>
+++ b/src/box/sequence.c<br>
@@ -196,7 +196,7 @@ sequence_next(struct sequence *seq, int64_t
*result)<br>
return -1;<br>
*result = def->start;<br>
if (txn_vdbe() != NULL &&<br>
- vdbe_add_new_generated_id(txn_vdbe(), *result) != 0)<br>
+ vdbe_add_new_autoinc_id(txn_vdbe(), *result) != 0)<br>
return -1;<br>
return 0;<br>
}<br>
@@ -233,7 +233,7 @@ done:<br>
unreachable();<br>
*result = value;<br>
if (txn_vdbe() != NULL &&<br>
- vdbe_add_new_generated_id(txn_vdbe(), value) != 0)<br>
+ vdbe_add_new_autoinc_id(txn_vdbe(), value) != 0)<br>
return -1;<br>
return 0;<br>
overflow:<br>
diff --git a/src/box/sequence.h b/src/box/sequence.h<br>
index 7180ead..4419427 100644<br>
--- a/src/box/sequence.h<br>
+++ b/src/box/sequence.h<br>
@@ -140,17 +140,8 @@ sequence_next(struct sequence *seq, int64_t
*result);<br>
int<br>
access_check_sequence(struct sequence *seq);<br>
<br>
-/**<br>
- * Append a new sequence value to Vdbe. List of automatically<br>
- * generated identifiers is returned as metadata of SQL response.<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>
+vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id);<br>
<br>
/**<br>
* Create an iterator over sequence data.<br>
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c<br>
index 1e732a9..dd763e7 100644<br>
--- a/src/box/sql/vdbe.c<br>
+++ b/src/box/sql/vdbe.c<br>
@@ -579,22 +579,24 @@ out2Prerelease(Vdbe *p, VdbeOp *pOp)<br>
}<br>
<br>
struct stailq *<br>
-vdbe_generated_ids(struct Vdbe *vdbe)<br>
+vdbe_autoinc_id_list(struct Vdbe *vdbe)<br>
{<br>
- return &vdbe->generated_ids;<br>
+ return &vdbe->autoinc_id_list;<br>
}<br>
<br>
int<br>
-vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id)<br>
+vdbe_add_new_autoinc_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>
+ struct autoinc_id_entry *id_entry =
region_alloc(&fiber()->gc,<br>
+ sizeof(*id_entry));<br>
if (id_entry == NULL) {<br>
- diag_set(OutOfMemory, sizeof(*id_entry), "region_alloc",
"id_entry");<br>
+ diag_set(OutOfMemory, sizeof(*id_entry), "region_alloc",<br>
+ "id_entry");<br>
return -1;<br>
}<br>
id_entry->id = id;<br>
- stailq_add_tail_entry(vdbe_generated_ids(vdbe), id_entry, link);<br>
+ stailq_add_tail_entry(vdbe_autoinc_id_list(vdbe), id_entry,
link);<br>
return 0;<br>
}<br>
<br>
@@ -3014,6 +3016,9 @@ case OP_TransactionBegin: {<br>
*<br>
* Commit Tarantool's transaction.<br>
* If there is no active transaction, raise an error.<br>
+ * After txn was commited VDBE should take care of region as<br>
+ * txn_commit() doesn't invoke fiber_gc(). Region is needed to<br>
+ * get information of autogenerated ids during sql response dump.<br>
*/<br>
case OP_TransactionCommit: {<br>
struct txn *txn = in_txn();<br>
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h<br>
index ed7fb2f..9546d42 100644<br>
--- a/src/box/sql/vdbe.h<br>
+++ b/src/box/sql/vdbe.h<br>
@@ -205,7 +205,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe);<br>
* @retval List of generated ids.<br>
*/<br>
struct stailq *<br>
-vdbe_generated_ids(struct Vdbe *vdbe);<br>
+vdbe_autoinc_id_list(struct Vdbe *vdbe);<br>
<br>
int sqlite3VdbeAddOp0(Vdbe *, int);<br>
int sqlite3VdbeAddOp1(Vdbe *, int, int);<br>
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h<br>
index 19b35b7..d16a51e 100644<br>
--- a/src/box/sql/vdbeInt.h<br>
+++ b/src/box/sql/vdbeInt.h<br>
@@ -369,7 +369,7 @@ struct Vdbe {<br>
* List of ids generated in current VDBE. It is returned<br>
* as metadata of SQL response.<br>
*/<br>
- struct stailq generated_ids;<br>
+ struct stailq autoinc_id_list;<br>
<br>
/* When allocating a new Vdbe object, all of the fields below
should be<br>
* initialized to zero or NULL<br>
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c<br>
index 3efe252..78cf2c4 100644<br>
--- a/src/box/sql/vdbeaux.c<br>
+++ b/src/box/sql/vdbeaux.c<br>
@@ -57,7 +57,7 @@ sqlite3VdbeCreate(Parse * pParse)<br>
return 0;<br>
memset(p, 0, sizeof(Vdbe));<br>
p->db = db;<br>
- stailq_create(&p->generated_ids);<br>
+ stailq_create(&p->autoinc_id_list);<br>
if (db->pVdbe) {<br>
db->pVdbe->pPrev = p;<br>
}<br>
@@ -2779,6 +2779,10 @@ sqlite3VdbeDelete(Vdbe * p)<br>
}<br>
p->magic = VDBE_MAGIC_DEAD;<br>
p->db = 0;<br>
+ /*<br>
+ * VDBE is responsible for releasing region after txn<br>
+ * was commited.<br>
+ */<br>
if (in_txn() == NULL)<br>
fiber_gc();<br>
sqlite3DbFree(db, p);<br>
diff --git a/src/box/txn.h b/src/box/txn.h<br>
index 87b55da..de5cb0d 100644<br>
--- a/src/box/txn.h<br>
+++ b/src/box/txn.h<br>
@@ -126,10 +126,8 @@ struct sql_txn {<br>
* An element of list of autogenerated ids, being returned as SQL<br>
* response metadata.<br>
*/<br>
-struct id_entry {<br>
- /** A link in a generated id list. */<br>
+struct autoinc_id_entry {<br>
struct stailq_entry link;<br>
- /** Generated id. */<br>
int64_t id;<br>
};<br>
<br>
diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>
index af4fc12..06cb4b9 100644<br>
--- a/test/sql/iproto.result<br>
+++ b/test/sql/iproto.result<br>
@@ -37,6 +37,9 @@ box.sql.execute('select * from test')<br>
box.schema.user.grant('guest','read,write,execute', 'universe')<br>
---<br>
...<br>
+box.schema.user.grant('guest', 'create', 'space')<br>
+---<br>
+...<br>
cn = remote.connect(box.cfg.listen)<br>
---<br>
...<br>
@@ -594,7 +597,7 @@ cn:execute('insert into test values (1, 1)')<br>
...<br>
cn:execute('insert into test values (null, 2)')<br>
---<br>
-- generated_ids:<br>
+- autoincrement_ids:<br>
- 2<br>
rowcount: 1<br>
...<br>
@@ -604,14 +607,14 @@ cn:execute('update test set a = 11 where id
== 1')<br>
...<br>
cn:execute('insert into test values (100, 1), (null, 1), (120,
1), (null, 1)')<br>
---<br>
-- generated_ids:<br>
+- autoincrement_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>
+- autoincrement_ids:<br>
- 122<br>
- 123<br>
- 124<br>
@@ -654,12 +657,27 @@ _ = box.space.TEST:on_replace(push_id)<br>
...<br>
cn:execute('insert into test values (null, 1)')<br>
---<br>
-- generated_ids:<br>
+- autoincrement_ids:<br>
- 127<br>
- 1<br>
- 2<br>
rowcount: 1<br>
...<br>
+box.sql.execute('create table test3 (id int primary key
autoincrement)')<br>
+---<br>
+...<br>
+box.schema.sequence.alter('TEST3', {min=-10000, step=-10})<br>
+---<br>
+...<br>
+cn:execute('insert into TEST3 values (null), (null), (null),
(null)')<br>
+---<br>
+- autoincrement_ids:<br>
+ - 1<br>
+ - -9<br>
+ - -19<br>
+ - -29<br>
+ rowcount: 4<br>
+...<br>
cn:close()<br>
---<br>
...<br>
@@ -672,9 +690,15 @@ s:drop()<br>
sq:drop()<br>
---<br>
...<br>
+box.sql.execute('drop table test3')<br>
+---<br>
+...<br>
box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
---<br>
...<br>
+box.schema.user.revoke('guest', 'create', 'space')<br>
+---<br>
+...<br>
space = nil<br>
---<br>
...<br>
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>
index a2caa57..51ac43b 100644<br>
--- a/test/sql/iproto.test.lua<br>
+++ b/test/sql/iproto.test.lua<br>
@@ -10,6 +10,7 @@ space:replace{4, 5, '6'}<br>
space:replace{7, 8.5, '9'}<br>
box.sql.execute('select * from test')<br>
box.schema.user.grant('guest','read,write,execute', 'universe')<br>
+box.schema.user.grant('guest', 'create', 'space')<br>
cn = remote.connect(box.cfg.listen)<br>
cn:ping()<br>
<br>
@@ -220,12 +221,18 @@ function push_id() s:replace{box.NULL}
s:replace{box.NULL} end<br>
_ = box.space.TEST:on_replace(push_id)<br>
cn:execute('insert into test values (null, 1)')<br>
<br>
+box.sql.execute('create table test3 (id int primary key
autoincrement)')<br>
+box.schema.sequence.alter('TEST3', {min=-10000, step=-10})<br>
+cn:execute('insert into TEST3 values (null), (null), (null),
(null)')<br>
+<br>
cn:close()<br>
box.sql.execute('drop table test')<br>
s:drop()<br>
sq:drop()<br>
+box.sql.execute('drop table test3')<br>
<br>
box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
+box.schema.user.revoke('guest', 'create', 'space')<br>
space = nil<br>
<br>
-- Cleanup xlog<br>
</p>
<p><b>New version:</b></p>
<p>commit 58f70a2cc6bfbed679f1601b09ba1a5d9db05872<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. After<br>
this patch all ids autogenerated during VDBE execution will be<br>
saved and returned through IPROTO.<br>
<br>
Closes #2618<br>
<br>
diff --git a/src/box/execute.c b/src/box/execute.c<br>
index 24459b4..64c0be6 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 *autoinc_id_list =<br>
+ vdbe_autoinc_id_list((struct Vdbe *)stmt);<br>
+ uint32_t map_size = stailq_empty(autoinc_id_list) ? 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(autoinc_id_list)) {<br>
+ struct autoinc_id_entry *id_entry;<br>
+ stailq_foreach_entry(id_entry, autoinc_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_AUTOINCREMENT_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(autoinc_id_list)) {<br>
+ buf = mp_encode_uint(buf,
SQL_INFO_AUTOINCREMENT_IDS);<br>
+ buf = mp_encode_array(buf, id_count);<br>
+ struct autoinc_id_entry *id_entry;<br>
+ stailq_foreach_entry(id_entry, autoinc_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>
}<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..77bfd79 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_AUTOINCREMENT_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..836ed6e 100644<br>
--- a/src/box/lua/net_box.c<br>
+++ b/src/box/lua/net_box.c<br>
@@ -668,16 +668,35 @@ 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_AUTOINCREMENT_IDS.<br>
+ */<br>
+ if (map_size == 2) {<br>
+ key = mp_decode_uint(data);<br>
+ assert(key == SQL_INFO_AUTOINCREMENT_IDS);<br>
+ (void)key;<br>
+ uint64_t count = mp_decode_array(data);<br>
+ assert(count > 0);<br>
+ lua_createtable(L, 0, count);<br>
+ for (uint32_t j = 0; j < count; ++j) {<br>
+ int64_t id;<br>
+ mp_read_int64(data, &id);<br>
+ luaL_pushint64(L, id);<br>
+ lua_rawseti(L, -2, j + 1);<br>
+ }<br>
+ lua_setfield(L, -2, "autoincrement_ids");<br>
+ }<br>
}<br>
<br>
static int<br>
diff --git a/src/box/sequence.c b/src/box/sequence.c<br>
index 35b7605..c9828c0 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_autoinc_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_autoinc_id(txn_vdbe(), value) != 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..4419427 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>
@@ -139,6 +140,9 @@ sequence_next(struct sequence *seq, int64_t
*result);<br>
int<br>
access_check_sequence(struct sequence *seq);<br>
<br>
+int<br>
+vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id);<br>
+<br>
/**<br>
* Create an iterator over sequence data.<br>
*<br>
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c<br>
index d6d6b7e..78fd28a 100644<br>
--- a/src/box/sql/vdbe.c<br>
+++ b/src/box/sql/vdbe.c<br>
@@ -578,6 +578,28 @@ out2Prerelease(Vdbe *p, VdbeOp *pOp)<br>
}<br>
}<br>
<br>
+struct stailq *<br>
+vdbe_autoinc_id_list(struct Vdbe *vdbe)<br>
+{<br>
+ return &vdbe->autoinc_id_list;<br>
+}<br>
+<br>
+int<br>
+vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id)<br>
+{<br>
+ assert(vdbe != NULL);<br>
+ struct autoinc_id_entry *id_entry =
region_alloc(&fiber()->gc,<br>
+ sizeof(*id_entry));<br>
+ if (id_entry == NULL) {<br>
+ diag_set(OutOfMemory, sizeof(*id_entry), "region_alloc",<br>
+ "id_entry");<br>
+ return -1;<br>
+ }<br>
+ id_entry->id = id;<br>
+ stailq_add_tail_entry(vdbe_autoinc_id_list(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>
@@ -2997,10 +3019,14 @@ case OP_TransactionBegin: {<br>
*<br>
* Commit Tarantool's transaction.<br>
* If there is no active transaction, raise an error.<br>
+ * After txn was commited VDBE should take care of region as<br>
+ * txn_commit() doesn't invoke fiber_gc(). Region is needed to<br>
+ * get information of autogenerated ids during sql response dump.<br>
*/<br>
case OP_TransactionCommit: {<br>
- if (box_txn()) {<br>
- if (box_txn_commit() != 0) {<br>
+ struct txn *txn = in_txn();<br>
+ if (txn != NULL) {<br>
+ if (txn_commit(txn) != 0) {<br>
rc = SQL_TARANTOOL_ERROR;<br>
goto abort_due_to_error;<br>
}<br>
@@ -3044,7 +3070,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..9546d42 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 v Vdbe with which associate this transaction.<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 *v);<br>
<br>
/**<br>
* Prepare given VDBE to execution: initialize structs connected<br>
@@ -197,6 +198,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_autoinc_id_list(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..cfb327a 100644<br>
--- a/src/box/sql/vdbeInt.h<br>
+++ b/src/box/sql/vdbeInt.h<br>
@@ -367,6 +367,11 @@ struct Vdbe {<br>
struct sql_txn *psql_txn;<br>
/** The auto-commit flag. */<br>
bool auto_commit;<br>
+ /**<br>
+ * List of ids generated in current VDBE. It is returned<br>
+ * as metadata of SQL response.<br>
+ */<br>
+ struct stailq autoinc_id_list;<br>
<br>
/* When allocating a new Vdbe object, all of the fields below
should be<br>
* initialized to zero or NULL<br>
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c<br>
index 6e42d05..1e72a4d 100644<br>
--- a/src/box/sql/vdbeaux.c<br>
+++ b/src/box/sql/vdbeaux.c<br>
@@ -57,6 +57,7 @@ sqlite3VdbeCreate(Parse * pParse)<br>
return 0;<br>
memset(p, 0, sizeof(Vdbe));<br>
p->db = db;<br>
+ stailq_create(&p->autoinc_id_list);<br>
if (db->pVdbe) {<br>
db->pVdbe->pPrev = p;<br>
}<br>
@@ -75,7 +76,7 @@ sqlite3VdbeCreate(Parse * pParse)<br>
}<br>
<br>
struct sql_txn *<br>
-sql_alloc_txn()<br>
+sql_alloc_txn(struct Vdbe *v)<br>
{<br>
struct sql_txn *txn =
region_alloc_object(&fiber()->gc,<br>
struct sql_txn);<br>
@@ -84,6 +85,8 @@ sql_alloc_txn()<br>
"struct sql_txn");<br>
return NULL;<br>
}<br>
+ txn->vdbe = v;<br>
+ v->psql_txn = txn;<br>
txn->pSavepoint = NULL;<br>
txn->fk_deferred_count = 0;<br>
return txn;<br>
@@ -103,11 +106,12 @@ sql_vdbe_prepare(struct Vdbe *vdbe)<br>
* check FK violations, at least now.<br>
*/<br>
if (txn->psql_txn == NULL) {<br>
- txn->psql_txn = sql_alloc_txn();<br>
+ txn->psql_txn = sql_alloc_txn(vdbe);<br>
if (txn->psql_txn == NULL)<br>
return -1;<br>
+ } else {<br>
+ vdbe->psql_txn = txn->psql_txn;<br>
}<br>
- vdbe->psql_txn = txn->psql_txn;<br>
} else {<br>
vdbe->psql_txn = NULL;<br>
}<br>
@@ -2261,12 +2265,11 @@ sql_txn_begin(Vdbe *p)<br>
struct txn *ptxn = txn_begin(false);<br>
if (ptxn == NULL)<br>
return -1;<br>
- ptxn->psql_txn = sql_alloc_txn();<br>
+ ptxn->psql_txn = sql_alloc_txn(p);<br>
if (ptxn->psql_txn == NULL) {<br>
box_txn_rollback();<br>
return -1;<br>
}<br>
- p->psql_txn = ptxn->psql_txn;<br>
return 0;<br>
}<br>
<br>
@@ -2414,9 +2417,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 +2783,12 @@ sqlite3VdbeDelete(Vdbe * p)<br>
}<br>
p->magic = VDBE_MAGIC_DEAD;<br>
p->db = 0;<br>
+ /*<br>
+ * VDBE is responsible for releasing region after txn<br>
+ * was commited.<br>
+ */<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..de5cb0d 100644<br>
--- a/src/box/txn.h<br>
+++ b/src/box/txn.h<br>
@@ -49,6 +49,7 @@ struct engine;<br>
struct space;<br>
struct tuple;<br>
struct xrow_header;<br>
+struct Vdbe;<br>
<br>
enum {<br>
/**<br>
@@ -117,6 +118,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>
+/**<br>
+ * An element of list of autogenerated ids, being returned as SQL<br>
+ * response metadata.<br>
+ */<br>
+struct autoinc_id_entry {<br>
+ struct stailq_entry link;<br>
+ int64_t id;<br>
};<br>
<br>
struct txn {<br>
@@ -337,6 +349,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 being currently executed.<br>
+ *<br>
+ * @retval VDBE that is being 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 ee34f6f..06cb4b9 100644<br>
--- a/test/sql/iproto.result<br>
+++ b/test/sql/iproto.result<br>
@@ -583,6 +583,116 @@ 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>
+- autoincrement_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>
+- autoincrement_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>
+- autoincrement_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>
+s = box.schema.create_space('test2', {engine = engine})<br>
+---<br>
+...<br>
+sq = box.schema.sequence.create('test2')<br>
+---<br>
+...<br>
+pk = s:create_index('pk', {sequence = 'test2'})<br>
+---<br>
+...<br>
+function push_id() s:replace{box.NULL} s:replace{box.NULL} end<br>
+---<br>
+...<br>
+_ = box.space.TEST:on_replace(push_id)<br>
+---<br>
+...<br>
+cn:execute('insert into test values (null, 1)')<br>
+---<br>
+- autoincrement_ids:<br>
+ - 127<br>
+ - 1<br>
+ - 2<br>
+ rowcount: 1<br>
+...<br>
+box.sql.execute('create table test3 (id int primary key
autoincrement)')<br>
+---<br>
+...<br>
+box.schema.sequence.alter('TEST3', {min=-10000, step=-10})<br>
+---<br>
+...<br>
+cn:execute('insert into TEST3 values (null), (null), (null),
(null)')<br>
+---<br>
+- autoincrement_ids:<br>
+ - 1<br>
+ - -9<br>
+ - -19<br>
+ - -29<br>
+ rowcount: 4<br>
+...<br>
+cn:close()<br>
+---<br>
+...<br>
+box.sql.execute('drop table test')<br>
+---<br>
+...<br>
+s:drop()<br>
+---<br>
+...<br>
+sq:drop()<br>
+---<br>
+...<br>
+box.sql.execute('drop table test3')<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 b065590..51ac43b 100644<br>
--- a/test/sql/iproto.test.lua<br>
+++ b/test/sql/iproto.test.lua<br>
@@ -202,6 +202,35 @@ 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>
+<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>
+<br>
+s = box.schema.create_space('test2', {engine = engine})<br>
+sq = box.schema.sequence.create('test2')<br>
+pk = s:create_index('pk', {sequence = 'test2'})<br>
+function push_id() s:replace{box.NULL} s:replace{box.NULL} end<br>
+_ = box.space.TEST:on_replace(push_id)<br>
+cn:execute('insert into test values (null, 1)')<br>
+<br>
+box.sql.execute('create table test3 (id int primary key
autoincrement)')<br>
+box.schema.sequence.alter('TEST3', {min=-10000, step=-10})<br>
+cn:execute('insert into TEST3 values (null), (null), (null),
(null)')<br>
+<br>
+cn:close()<br>
+box.sql.execute('drop table test')<br>
+s:drop()<br>
+sq:drop()<br>
+box.sql.execute('drop table test3')<br>
+<br>
box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
box.schema.user.revoke('guest', 'create', 'space')<br>
space = nil<br>
<br>
</p>
</body>
</html>