[tarantool-patches] [PATCH v1 1/1] sql: return last_insert_id via IPROTO

[imeevma@tarantool.org]

After this patch client will get last_insert_id
as additional metadata when he executes some
queries.

Part of #2618.
---
Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-2618-return-generated-columns-and-values
Issue: https://github.com/tarantool/tarantool/issues/2618

 src/box/execute.c             |  12 +++-
 src/box/execute.h             |   1 +
 src/box/lua/net_box.c         |  15 +++-
 src/box/sequence.c            |  13 ++++
 src/box/sequence.h            |   9 +++
 src/box/session.cc            |   1 +
 src/box/session.h             |   2 +
 src/box/sql.c                 |  17 +++++
 src/box/sql/func.c            |  19 +++++
 src/box/sql/sqliteInt.h       |  18 +++++
 src/box/sql/vdbe.c            |  14 +++-
 test/sql-tap/insert3.test.lua |  27 ++++++-
 test/sql/errinj.result        |   3 +-
 test/sql/iproto.result        | 162 +++++++++++++++++++++++++++++++++---------
 test/sql/iproto.test.lua      |  16 +++++
 15 files changed, 289 insertions(+), 40 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index 24459b4..f2b31c6 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -640,8 +640,16 @@ err:
 		if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0)
 			goto err;
 		int changes = sqlite3_changes(db);
+		/*
+		 * Even though last_insert_id is int64, it
+		 * is easier to work with it as uint64.
+		 */
+		uint64_t last_insert_id = (uint64_t)get_last_insert_id();
+
 		int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +
-			   mp_sizeof_uint(changes);
+			   mp_sizeof_uint(changes) +
+			   mp_sizeof_uint(SQL_INFO_LAST_INSERT_ID) +
+			   mp_sizeof_uint(last_insert_id);
 		char *buf = obuf_alloc(out, size);
 		if (buf == NULL) {
 			diag_set(OutOfMemory, size, "obuf_alloc", "buf");
@@ -649,6 +657,8 @@ err:
 		}
 		buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT);
 		buf = mp_encode_uint(buf, changes);
+		buf = mp_encode_uint(buf, SQL_INFO_LAST_INSERT_ID);
+		buf = mp_encode_uint(buf, last_insert_id);
 	}
 	iproto_reply_sql(out, &header_svp, response->sync, schema_version,
 			 keys);
diff --git a/src/box/execute.h b/src/box/execute.h
index f21393b..ead09fc 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_LAST_INSERT_ID = 1,
 	sql_info_key_MAX,
 };
 
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 308c9c7..297b664 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -667,16 +667,25 @@ static void
 netbox_decode_sql_info(struct lua_State *L, const char **data)
 {
 	uint32_t map_size = mp_decode_map(data);
-	/* Only SQL_INFO_ROW_COUNT is available. */
 	assert(map_size == 1);
 	(void) map_size;
 	uint32_t key = mp_decode_uint(data);
 	assert(key == SQL_INFO_ROW_COUNT);
-	(void) key;
 	uint32_t row_count = mp_decode_uint(data);
-	lua_createtable(L, 0, 1);
+	key = mp_decode_uint(data);
+	assert(key == SQL_INFO_LAST_INSERT_ID);
+	(void) key;
+	/*
+	 * In our case last_insert_id is int64 and we know that
+	 * it was converted to uint64 for convenience. Now we
+	 * should convert it back.
+	 */
+	int64_t last_insert_id = (int64_t)mp_decode_uint(data);
+	lua_createtable(L, 0, 2);
 	lua_pushinteger(L, row_count);
 	lua_setfield(L, -2, "rowcount");
+	lua_pushinteger(L, last_insert_id);
+	lua_setfield(L, -2, "last_insert_id");
 }
 
 static int
diff --git a/src/box/sequence.c b/src/box/sequence.c
index 35b7605..fe66931 100644
--- a/src/box/sequence.c
+++ b/src/box/sequence.c
@@ -340,3 +340,16 @@ sequence_data_iterator_create(void)
 	light_sequence_iterator_freeze(&sequence_data_index, &iter->iter);
 	return &iter->base;
 }
+
+int64_t
+sequence_get_value(struct sequence *seq)
+{
+	uint32_t key = seq->def->id;
+	uint32_t hash = sequence_hash(key);
+	uint32_t pos = light_sequence_find_key(&sequence_data_index, hash, key);
+	if (pos == light_sequence_end)
+		return 0;
+	struct sequence_data data = light_sequence_get(&sequence_data_index,
+						       pos);
+	return data.value;
+}
diff --git a/src/box/sequence.h b/src/box/sequence.h
index 0f6c8da..c1a746e 100644
--- a/src/box/sequence.h
+++ b/src/box/sequence.h
@@ -149,6 +149,15 @@ access_check_sequence(struct sequence *seq);
 struct snapshot_iterator *
 sequence_data_iterator_create(void);
 
+/**
+ * Get current value of given sequence.
+ *
+ * @param seq sequence to get value from.
+ * @retval current value of sequence.
+ */
+int64_t
+sequence_get_value(struct sequence *seq);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/src/box/session.cc b/src/box/session.cc
index 64714cd..f19853e 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -108,6 +108,7 @@ session_create(enum session_type type)
 	session->type = type;
 	session->sql_flags = default_flags;
 	session->sql_default_engine = SQL_STORAGE_ENGINE_MEMTX;
+	session->last_insert_id = 0;
 
 	/* For on_connect triggers. */
 	credentials_init(&session->credentials, guest_user->auth_token,
diff --git a/src/box/session.h b/src/box/session.h
index df1dcbc..c9a0622 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -103,6 +103,8 @@ struct session {
 	struct credentials credentials;
 	/** Trigger for fiber on_stop to cleanup created on-demand session */
 	struct trigger fiber_on_stop;
+	/** ID of first inserted tuple in last SQL INSERT query */
+	int64_t last_insert_id;
 };
 
 struct session_vtab {
diff --git a/src/box/sql.c b/src/box/sql.c
index d48c3cf..bb275eb 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -48,6 +48,7 @@
 #include "txn.h"
 #include "space.h"
 #include "space_def.h"
+#include "sequence.h"
 #include "index_def.h"
 #include "tuple.h"
 #include "fiber.h"
@@ -1794,3 +1795,19 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
 	sql_parser_destroy(&parser);
 	return rc;
 }
+
+void
+set_last_insert_id(struct space *space)
+{
+	struct session *session = current_session();
+	session->last_insert_id = 0;
+	struct sequence *sequence = space->sequence;
+	if(sequence != NULL)
+		session->last_insert_id = sequence_get_value(sequence);
+}
+
+int64_t
+get_last_insert_id(void)
+{
+	return current_session()->last_insert_id;
+}
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index e211de1..b744aa3 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -37,6 +37,7 @@
 #include "sqliteInt.h"
 #include "vdbeInt.h"
 #include "version.h"
+#include "src/box/session.h"
 #include <unicode/ustring.h>
 #include <unicode/ucasemap.h>
 #include <unicode/ucnv.h>
@@ -591,6 +592,23 @@ changes(sqlite3_context * context, int NotUsed, sqlite3_value ** NotUsed2)
 }
 
 /*
+ * Implementation of the last_insert_id() function. Returns last
+ * autogenerated ID of first inserted tuple in last insert in
+ * current session.
+ *
+ * @param context Context being used.
+ * @param not_used Unused.
+ * @param not_used2 Unused.
+ */
+static void
+last_insert_id(sqlite3_context *context, int not_used,
+	       sqlite3_value **not_used2)
+{
+	UNUSED_PARAMETER2(not_used, not_used2);
+	sqlite3_result_int(context, get_last_insert_id());
+}
+
+/*
  * Implementation of the total_changes() SQL function.  The return value is
  * the same as the sqlite3_total_changes() API function.
  */
@@ -1839,6 +1857,7 @@ sqlite3RegisterBuiltinFunctions(void)
 		FUNCTION(lower, 1, 0, 0, LowerICUFunc),
 		FUNCTION(hex, 1, 0, 0, hexFunc),
 		FUNCTION2(ifnull, 2, 0, 0, noopFunc, SQLITE_FUNC_COALESCE),
+		VFUNCTION(last_insert_id, 0, 0, 0, last_insert_id),
 		VFUNCTION(random, 0, 0, 0, randomFunc),
 		VFUNCTION(randomblob, 1, 0, 0, randomBlob),
 		FUNCTION(nullif, 2, 0, 1, nullifFunc),
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index b595984..c204a61 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4863,4 +4863,22 @@ void
 vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
 			   const char *idx_name, const char *table_name);
 
+/**
+ * Set last_insert_id of current session as last autogenerated ID
+ * of first inserted tuple in last INSERT query executed in
+ * current session.
+ *
+ * @param space space to get sequence from.
+ */
+void
+set_last_insert_id(struct space *space);
+
+/**
+ * Get last_insert_id of current session.
+ *
+ * @retval last_insert_id last_insert_id of current session.
+ */
+int64_t
+get_last_insert_id(void);
+
 #endif				/* SQLITEINT_H */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 2144d95..0762b97 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -599,6 +599,11 @@ int sqlite3VdbeExec(Vdbe *p)
 	u64 start;                 /* CPU clock count at start of opcode */
 #endif
 	struct session *user_session = current_session();
+	/*
+	 * Field last_insert_id of current session should be set
+	 * no more than once for each query.
+	 */
+	bool last_insert_id_is_set = false;
 	/*** INSERT STACK UNION HERE ***/
 
 	assert(p->magic==VDBE_MAGIC_RUN);  /* sqlite3_step() verifies this */
@@ -4335,14 +4340,19 @@ case OP_IdxInsert: {        /* in2 */
 		if (pBtCur->curFlags & BTCF_TaCursor) {
 			/* Make sure that memory has been allocated on region. */
 			assert(aMem[pOp->p2].flags & MEM_Ephem);
-			if (pOp->opcode == OP_IdxInsert)
+			if (pOp->opcode == OP_IdxInsert) {
 				rc = tarantoolSqlite3Insert(pBtCur->space,
 							    pIn2->z,
 							    pIn2->z + pIn2->n);
-			else
+				if (rc == 0 && !last_insert_id_is_set) {
+					set_last_insert_id(pBtCur->space);
+					last_insert_id_is_set = true;
+				}
+			} else {
 				rc = tarantoolSqlite3Replace(pBtCur->space,
 							     pIn2->z,
 							     pIn2->z + pIn2->n);
+			}
 		} else if (pBtCur->curFlags & BTCF_TEphemCursor) {
 			rc = tarantoolSqlite3EphemeralInsert(pBtCur->space,
 							     pIn2->z,
diff --git a/test/sql-tap/insert3.test.lua b/test/sql-tap/insert3.test.lua
index c0e9d95..8394cd9 100755
--- a/test/sql-tap/insert3.test.lua
+++ b/test/sql-tap/insert3.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(18)
+test:plan(20)
 
 --!./tcltestrunner.lua
 -- 2005 January 13
@@ -262,6 +262,31 @@ test:do_execsql_test(
 		-- <insert3-4.1>
 })
 
+-- gh-2618 function last_insert_id
+test:execsql("CREATE TABLE t8(id INTEGER PRIMARY KEY AUTOINCREMENT, a INT);")
+test:do_execsql_test(
+    "insert3-5.1",
+    [[
+        INSERT INTO t8 VALUES (null, 11);
+        SELECT LAST_INSERT_ID();
+    ]], {
+        -- <insert3-5.1>
+        1
+        -- <insert3-5.1>
+})
+
+test:do_execsql_test(
+    "insert3-5.2",
+    [[
+        INSERT INTO t8 VALUES (null, 44), (null, 55), (null, 66);
+        SELECT LAST_INSERT_ID();
+    ]], {
+        -- <insert3-5.1>
+        2
+        -- <insert3-5.1>
+})
+
+
 test:drop_all_tables()
 ---------------------------------------------------------------------------
 -- While developing tests for a different feature (savepoint) the following
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index b491d46..011b21f 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -73,7 +73,8 @@ while f1:status() ~= 'dead' do fiber.sleep(0) end
 ...
 insert_res
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 select_res
 ---
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index af474bc..4e18435 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -69,19 +69,23 @@ type(ret.rows[1])
 -- Operation with rowcount result.
 cn:execute('insert into test values (10, 11, NULL)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('delete from test where a = 5')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('insert into test values (11, 12, NULL), (12, 12, NULL), (13, 12, NULL)')
 ---
-- rowcount: 3
+- last_insert_id: 0
+  rowcount: 3
 ...
 cn:execute('delete from test where a = 12')
 ---
-- rowcount: 3
+- last_insert_id: 0
+  rowcount: 3
 ...
 -- SQL errors.
 cn:execute('insert into not_existing_table values ("kek")')
@@ -330,7 +334,8 @@ cn:execute('select :value', parameters)
 -- gh-2608 SQL iproto DDL
 cn:execute('create table test2(id primary key, a, b, c)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 box.space.TEST2.name
 ---
@@ -338,7 +343,8 @@ box.space.TEST2.name
 ...
 cn:execute('insert into test2 values (1, 1, 1, 1)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('select * from test2')
 ---
@@ -352,7 +358,8 @@ cn:execute('select * from test2')
 ...
 cn:execute('create index test2_a_b_index on test2(a, b)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 #box.space.TEST2.index
 ---
@@ -360,7 +367,8 @@ cn:execute('create index test2_a_b_index on test2(a, b)')
 ...
 cn:execute('drop table test2')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 box.space.TEST2
 ---
@@ -370,100 +378,121 @@ box.space.TEST2
 -- Test CREATE [IF NOT EXISTS] TABLE.
 cn:execute('create table test3(id primary key, a, b)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 -- Rowcount = 1, although two tuples were created:
 -- for _space and for _index.
 cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)')
 ---
-- rowcount: 3
+- last_insert_id: 0
+  rowcount: 3
 ...
 cn:execute('create table if not exists test3(id primary key)')
 ---
-- rowcount: 0
+- last_insert_id: 0
+  rowcount: 0
 ...
 -- Test CREATE VIEW [IF NOT EXISTS] and
 --      DROP   VIEW [IF EXISTS].
 cn:execute('create view test3_view(id) as select id from test3')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('create view if not exists test3_view(id) as select id from test3')
 ---
-- rowcount: 0
+- last_insert_id: 0
+  rowcount: 0
 ...
 cn:execute('drop view test3_view')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('drop view if exists test3_view')
 ---
-- rowcount: 0
+- last_insert_id: 0
+  rowcount: 0
 ...
 -- Test CREATE INDEX [IF NOT EXISTS] and
 --      DROP   INDEX [IF EXISTS].
 cn:execute('create index test3_sec on test3(a, b)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('create index if not exists test3_sec on test3(a, b)')
 ---
-- rowcount: 0
+- last_insert_id: 0
+  rowcount: 0
 ...
 cn:execute('drop index test3_sec on test3')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('drop index if exists test3_sec on test3')
 ---
-- rowcount: 0
+- last_insert_id: 0
+  rowcount: 0
 ...
 -- Test CREATE TRIGGER [IF NOT EXISTS] and
 --      DROP   TRIGGER [IF EXISTS].
 cn:execute('create trigger trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('create trigger if not exists trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 ---
-- rowcount: 0
+- last_insert_id: 0
+  rowcount: 0
 ...
 cn:execute('drop trigger trig')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('drop trigger if exists trig')
 ---
-- rowcount: 0
+- last_insert_id: 0
+  rowcount: 0
 ...
 -- Test DROP TABLE [IF EXISTS].
 -- Create more indexes, triggers and _truncate tuple.
 cn:execute('create index idx1 on test3(a)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('create index idx2 on test3(b)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 box.space.TEST3:truncate()
 ---
 ...
 cn:execute('create trigger trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)')
 ---
-- rowcount: 3
+- last_insert_id: 0
+  rowcount: 3
 ...
 cn:execute('drop table test3')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 cn:execute('drop table if exists test3')
 ---
-- rowcount: 0
+- last_insert_id: 0
+  rowcount: 0
 ...
 --
 -- gh-2948: sql: remove unnecessary templates for binding
@@ -535,7 +564,8 @@ cn = remote.connect(box.cfg.listen)
 ...
 cn:execute('create table test (id integer primary key, a integer, b integer)')
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 future1 = cn:execute('insert into test values (1, 1, 1)', nil, nil, {is_async = true})
 ---
@@ -548,7 +578,8 @@ future3 = cn:execute('insert into test values (2, 2, 2), (3, 3, 3)', nil, nil, {
 ...
 future1:wait_result()
 ---
-- rowcount: 1
+- last_insert_id: 0
+  rowcount: 1
 ...
 future2:wait_result()
 ---
@@ -558,7 +589,8 @@ future2:wait_result()
 ...
 future3:wait_result()
 ---
-- rowcount: 2
+- last_insert_id: 0
+  rowcount: 2
 ...
 future4 = cn:execute('select * from test', nil, nil, {is_async = true})
 ---
@@ -580,6 +612,72 @@ cn:close()
 box.sql.execute('drop table test')
 ---
 ...
+-- gh-2618 Return generated columns after INSERT in IPROTO.
+-- Return autogenerated id of first inserted tuple in last insert.
+box.sql.execute('create table test (id integer primary key autoincrement, a integer)')
+---
+...
+cn = remote.connect(box.cfg.listen)
+---
+...
+cn:execute('insert into test values (1, 1)', nil, nil)
+---
+- last_insert_id: 1
+  rowcount: 1
+...
+cn:execute('insert into test values (null, 2)', nil, nil)
+---
+- last_insert_id: 2
+  rowcount: 1
+...
+cn:execute('insert into test values (null, 2), (null, 3)', nil, nil)
+---
+- last_insert_id: 3
+  rowcount: 2
+...
+cn:execute('insert into test values (17, 2)', nil, nil)
+---
+- last_insert_id: 17
+  rowcount: 1
+...
+cn:execute('insert into test values (null, 2), (null, 3)', nil, nil)
+---
+- last_insert_id: 18
+  rowcount: 2
+...
+future = cn:execute('select * from test', nil, nil, {is_async = true})
+---
+...
+future:wait_result()
+---
+- metadata:
+  - name: ID
+  - name: A
+  rows:
+  - [1, 1]
+  - [2, 2]
+  - [3, 2]
+  - [4, 3]
+  - [17, 2]
+  - [18, 2]
+  - [19, 3]
+...
+cn:execute('create table test2 (id integer primary key, a integer)')
+---
+- last_insert_id: 18
+  rowcount: 1
+...
+cn:execute('drop table test2')
+---
+- last_insert_id: 18
+  rowcount: 1
+...
+cn:close()
+---
+...
+box.sql.execute('drop table test')
+---
+...
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 ---
 ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 220331b..033340c 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -201,6 +201,22 @@ future4:wait_result()
 cn:close()
 box.sql.execute('drop table test')
 
+-- gh-2618 Return generated columns after INSERT in IPROTO.
+-- Return autogenerated id of first inserted tuple in last insert.
+box.sql.execute('create table test (id integer primary key autoincrement, a integer)')
+cn = remote.connect(box.cfg.listen)
+cn:execute('insert into test values (1, 1)', nil, nil)
+cn:execute('insert into test values (null, 2)', nil, nil)
+cn:execute('insert into test values (null, 2), (null, 3)', nil, nil)
+cn:execute('insert into test values (17, 2)', nil, nil)
+cn:execute('insert into test values (null, 2), (null, 3)', nil, nil)
+future = cn:execute('select * from test', nil, nil, {is_async = true})
+future:wait_result()
+cn:execute('create table test2 (id integer primary key, a integer)')
+cn:execute('drop table test2')
+cn:close()
+box.sql.execute('drop table test')
+
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 space = nil
 
-- 
2.7.4