[tarantool-patches] [PATCH v3 0/2] sql: return last_insert_id via IPROTO

[tarantool-patches] [PATCH v3 0/2] sql: return last_insert_id via IPROTO

[imeevma]

This patch-set add new data to metadata returned via IPROTO
after some SQL statements.

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

Mergen Imeev (2):
  sql: move autoincrement in vdbe
  sql: return last_insert_id via IPROTO

 src/box/execute.c                       |  11 +-
 src/box/execute.h                       |   1 +
 src/box/lua/net_box.c                   |  10 +-
 src/box/session.cc                      |   1 +
 src/box/session.h                       |   5 +
 src/box/sql/func.c                      |  19 ++++
 src/box/sql/insert.c                    |  13 ++-
 src/box/sql/vdbe.c                      |  18 +++-
 src/box/sql/vdbeInt.h                   |   5 +
 test/sql-tap/insert3.test.lua           |  27 ++++-
 test/sql/errinj.result                  |   3 +-
 test/sql/gh-2981-check-autoinc.result   |  13 +++
 test/sql/gh-2981-check-autoinc.test.lua |   6 +-
 test/sql/iproto.result                  | 181 ++++++++++++++++++++++++++------
 test/sql/iproto.test.lua                |  20 ++++
 15 files changed, 289 insertions(+), 44 deletions(-)

-- 
2.7.4

[tarantool-patches] [PATCH v3 1/2] sql: move autoincrement in vdbe

[imeevma]

This patch expands changes made in issue #2981. Now NULL
in primary key always replaced by generated value inside
of vdbe. It allows us to get last_insert_id with easy.

Part of #2618
---
 src/box/sql/insert.c                    | 13 +++++++++----
 src/box/sql/vdbe.c                      |  6 +++++-
 test/sql/gh-2981-check-autoinc.result   | 13 +++++++++++++
 test/sql/gh-2981-check-autoinc.test.lua |  6 +++++-
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 0e92f62..6afa0a2 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -739,9 +739,7 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 			if (j < 0 || nColumn == 0
 			    || (pColumn && j >= pColumn->nId)) {
 				if (i == pTab->iAutoIncPKey) {
-					sqlite3VdbeAddOp2(v,
-							  OP_NextAutoincValue,
-							  pTab->def->id,
+					sqlite3VdbeAddOp2(v, OP_Null, 0,
 							  iRegStore);
 					continue;
 				}
@@ -824,7 +822,14 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 						iRegStore);
 			}
 		}
-
+		/*
+		 * Replace NULL in PK with autoincrement by
+		 * generated value.
+		 */
+		if (pTab->iAutoIncPKey >= 0) {
+			sqlite3VdbeAddOp2(v, OP_NextAutoincValue, pTab->def->id,
+					  regData + pTab->iAutoIncPKey);
+		}
 		/* Generate code to check constraints and generate index keys
 		   and do the insertion.
 		 */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index dc3f5c0..c6ece15 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1157,6 +1157,10 @@ case OP_NextAutoincValue: {
 	assert(pOp->p1 > 0);
 	assert(pOp->p2 > 0);
 
+	pIn2 = &p->aMem[pOp->p2];
+	if ((pIn2->flags & MEM_Null) == 0)
+		break;
+
 	struct space *space = space_by_id(pOp->p1);
 	if (space == NULL) {
 		rc = SQL_TARANTOOL_ERROR;
@@ -3755,7 +3759,7 @@ case OP_FCopy: {     /* out2 */
 
 	if ((pOp->p3 & OPFLAG_NOOP_IF_NULL) && (pIn1->flags & MEM_Null)) {
 		pOut = &aMem[pOp->p2];
-		if (pOut->flags & MEM_Undefined) pOut->flags = MEM_Null;
+		pOut->flags = MEM_Null;
 		/* Flag is set and register is NULL -> do nothing  */
 	} else {
 		assert(memIsValid(pIn1));
diff --git a/test/sql/gh-2981-check-autoinc.result b/test/sql/gh-2981-check-autoinc.result
index b0f55e6..43bd42b 100644
--- a/test/sql/gh-2981-check-autoinc.result
+++ b/test/sql/gh-2981-check-autoinc.result
@@ -19,6 +19,9 @@ box.sql.execute("CREATE TABLE t2 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEG
 box.sql.execute("CREATE TABLE t3 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 < 10));");
 ---
 ...
+box.sql.execute("CREATE TABLE t4 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
+---
+...
 box.sql.execute("insert into t1 values (18, null);")
 ---
 ...
@@ -47,6 +50,13 @@ box.sql.execute("insert into t3(s2) values (null)")
 ---
 - error: 'CHECK constraint failed: T3'
 ...
+box.sql.execute("insert into t4 values (18, null);")
+---
+...
+box.sql.execute("insert into t4 values (null, null);")
+---
+- error: 'CHECK constraint failed: T4'
+...
 box.sql.execute("DROP TABLE t1")
 ---
 ...
@@ -56,3 +66,6 @@ box.sql.execute("DROP TABLE t2")
 box.sql.execute("DROP TABLE t3")
 ---
 ...
+box.sql.execute("DROP TABLE t4")
+---
+...
diff --git a/test/sql/gh-2981-check-autoinc.test.lua b/test/sql/gh-2981-check-autoinc.test.lua
index 98a5fb4..593ff3a 100644
--- a/test/sql/gh-2981-check-autoinc.test.lua
+++ b/test/sql/gh-2981-check-autoinc.test.lua
@@ -7,6 +7,7 @@ box.cfg{}
 box.sql.execute("CREATE TABLE t1 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
 box.sql.execute("CREATE TABLE t2 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19 AND s1 <> 25));");
 box.sql.execute("CREATE TABLE t3 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 < 10));");
+box.sql.execute("CREATE TABLE t4 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
 
 box.sql.execute("insert into t1 values (18, null);")
 box.sql.execute("insert into t1(s2) values (null);")
@@ -19,7 +20,10 @@ box.sql.execute("insert into t2(s2) values (null);")
 box.sql.execute("insert into t3 values (9, null)")
 box.sql.execute("insert into t3(s2) values (null)")
 
+box.sql.execute("insert into t4 values (18, null);")
+box.sql.execute("insert into t4 values (null, null);")
+
 box.sql.execute("DROP TABLE t1")
 box.sql.execute("DROP TABLE t2")
 box.sql.execute("DROP TABLE t3")
-
+box.sql.execute("DROP TABLE t4")
-- 
2.7.4

[tarantool-patches] [PATCH v3 2/2] sql: return last_insert_id via IPROTO

[imeevma]

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

Part of #2618

@TarantoolBot document
Title: SQL function last_insert_id() and IPROTO key last_insert_id.
Function last_insert_id() returns first primary key value
autogenerated in last INSERT/REPLACE statement in current
session. User can have more than one session and this
function will work properly for each one of them. Return
value of function is undetermined when more than one
INSERT/REPLACE statements executed asynchronously.
IPROTO key last_insert_id is a metadata returned through
IPROTO after such statements as INSERT, REPLACE, UPDATE
etc. Value of this key is equal to value returned by
function last_insert_id() executed after the statement.
Example:
CREATE TABLE test (id INTEGER PRIMARY KEY AUTOINCREMENT, a INTEGER);
INSERT INTO test VALUES (NULL, 1);
SELECT last_insert_id();
---
 src/box/execute.c             |  11 ++-
 src/box/execute.h             |   1 +
 src/box/lua/net_box.c         |  10 ++-
 src/box/session.cc            |   1 +
 src/box/session.h             |   5 ++
 src/box/sql/func.c            |  19 +++++
 src/box/sql/vdbe.c            |  12 +++
 src/box/sql/vdbeInt.h         |   5 ++
 test/sql-tap/insert3.test.lua |  27 ++++++-
 test/sql/errinj.result        |   3 +-
 test/sql/iproto.result        | 181 ++++++++++++++++++++++++++++++++++--------
 test/sql/iproto.test.lua      |  20 +++++
 12 files changed, 257 insertions(+), 38 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index 24459b4..e2eabc7 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 "session.h"
 
 const char *sql_type_strs[] = {
 	NULL,
@@ -640,8 +641,13 @@ err:
 		if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0)
 			goto err;
 		int changes = sqlite3_changes(db);
+		int64_t last_insert_id = current_session()->sql_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) +
+			   (last_insert_id >= 0 ?
+			    mp_sizeof_uint(last_insert_id) :
+			    mp_sizeof_int(last_insert_id));
 		char *buf = obuf_alloc(out, size);
 		if (buf == NULL) {
 			diag_set(OutOfMemory, size, "obuf_alloc", "buf");
@@ -649,6 +655,9 @@ 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 = last_insert_id < 0 ? mp_encode_int(buf, last_insert_id) :
+		      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..4a82fca 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -667,16 +667,20 @@ 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);
+	int64_t last_insert_id;
+	mp_read_int64(data, &last_insert_id);
+	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/session.cc b/src/box/session.cc
index 64714cd..2b8dab9 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->sql_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..6ee22bc 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -92,6 +92,11 @@ union session_meta {
 struct session {
 	/** Session id. */
 	uint64_t id;
+	/**
+	 * First primary key autogenerated in last INSERT/REPLACE
+	 * statement in which primary key was generated.
+	 */
+	int64_t sql_last_insert_id;
 	/** SQL Tarantool Default storage engine. */
 	uint8_t sql_default_engine;
 	/** SQL Connection flag for current user session */
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 45056a7..2607cc3 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -38,6 +38,7 @@
 #include "vdbeInt.h"
 #include "version.h"
 #include "coll.h"
+#include "box/session.h"
 #include <unicode/ustring.h>
 #include <unicode/ucasemap.h>
 #include <unicode/ucnv.h>
@@ -601,6 +602,23 @@ changes(sqlite3_context * context, int NotUsed, sqlite3_value ** NotUsed2)
 }
 
 /*
+ * Return first primary key autogenerated in last INSERT/REPLACE
+ * statement in which primary key was generated 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, current_session()->sql_last_insert_id);
+}
+
+/*
  * Implementation of the total_changes() SQL function.  The return value is
  * the same as the sqlite3_total_changes() API function.
  */
@@ -1847,6 +1865,7 @@ sqlite3RegisterBuiltinFunctions(void)
 		FUNCTION(lower, 1, 0, 1, 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/vdbe.c b/src/box/sql/vdbe.c
index c6ece15..93ef00b 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -610,6 +610,11 @@ int sqlite3VdbeExec(Vdbe *p)
 	u64 start;                 /* CPU clock count at start of opcode */
 #endif
 	struct session *user_session = current_session();
+	/*
+	 * Field sql_last_insert_id of current session should be
+	 * set no more than once for each statement.
+	 */
+	p->is_last_insert_id_set = false;
 	/*** INSERT STACK UNION HERE ***/
 
 	assert(p->magic==VDBE_MAGIC_RUN);  /* sqlite3_step() verifies this */
@@ -1178,6 +1183,13 @@ case OP_NextAutoincValue: {
 	pOut->flags = MEM_Int;
 	pOut->u.i = value;
 
+	/* Set sql_last_insert_id of current session. */
+	if (!p->is_last_insert_id_set) {
+		struct session *session = current_session();
+		session->sql_last_insert_id = value;
+		p->is_last_insert_id_set = true;
+	}
+
 	break;
 }
 
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index ce97f49..42ccdeb 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -393,6 +393,11 @@ struct Vdbe {
 	bft changeCntOn:1;	/* True to update the change-counter */
 	bft runOnlyOnce:1;	/* Automatically expire on reset */
 	bft isPrepareV2:1;	/* True if prepared with prepare_v2() */
+	/*
+	 * True if field last_insert_id of current session was set
+	 * in current statement.
+	 */
+	bool is_last_insert_id_set : 1;
 	u32 aCounter[5];	/* Counters used by sqlite3_stmt_status() */
 	char *zSql;		/* Text of the SQL statement that generated this */
 	void *pFree;		/* Free this when deleting the vdbe */
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 a0ba60f..9443137 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..4e3cfe1 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,91 @@ cn:close()
 box.sql.execute('drop table test')
 ---
 ...
+-- gh-2618 Return generated columns after INSERT in IPROTO.
+-- Return first PK autogenerated in last INSERT/REPLACE in which
+-- new PK was genereted in current session.
+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)')
+---
+- last_insert_id: 0
+  rowcount: 1
+...
+cn:execute('insert into test values (null, 2)')
+---
+- last_insert_id: 2
+  rowcount: 1
+...
+cn:execute('insert into test values (null, 2), (null, 3)')
+---
+- last_insert_id: 3
+  rowcount: 2
+...
+cn:execute('insert into test values (17, 2)')
+---
+- last_insert_id: 3
+  rowcount: 1
+...
+cn:execute('update test set a = 11 where id == 1')
+---
+- last_insert_id: 3
+  rowcount: 1
+...
+cn:execute('update test set a = 12 where id == 2')
+---
+- last_insert_id: 3
+  rowcount: 1
+...
+cn:execute('update test set a = 13 where id == 3')
+---
+- last_insert_id: 3
+  rowcount: 1
+...
+cn:execute('replace into test values (4, 44), (null, 100)')
+---
+- last_insert_id: 18
+  rowcount: 2
+...
+cn:execute('insert into test values (null, 2), (null, 3)')
+---
+- last_insert_id: 19
+  rowcount: 2
+...
+cn:execute('select * from test')
+---
+- metadata:
+  - name: ID
+  - name: A
+  rows:
+  - [1, 11]
+  - [2, 12]
+  - [3, 13]
+  - [4, 44]
+  - [17, 2]
+  - [18, 100]
+  - [19, 2]
+  - [20, 3]
+...
+cn:execute('create table test2 (id integer primary key, a integer)')
+---
+- last_insert_id: 19
+  rowcount: 1
+...
+cn:execute('drop table test2')
+---
+- last_insert_id: 19
+  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..a9f55fb 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -201,6 +201,26 @@ future4:wait_result()
 cn:close()
 box.sql.execute('drop table test')
 
+-- gh-2618 Return generated columns after INSERT in IPROTO.
+-- Return first PK autogenerated in last INSERT/REPLACE in which
+-- new PK was genereted in current session.
+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)')
+cn:execute('insert into test values (null, 2)')
+cn:execute('insert into test values (null, 2), (null, 3)')
+cn:execute('insert into test values (17, 2)')
+cn:execute('update test set a = 11 where id == 1')
+cn:execute('update test set a = 12 where id == 2')
+cn:execute('update test set a = 13 where id == 3')
+cn:execute('replace into test values (4, 44), (null, 100)')
+cn:execute('insert into test values (null, 2), (null, 3)')
+cn:execute('select * from test')
+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

[tarantool-patches] Re: [PATCH v3 2/2] sql: return last_insert_id via IPROTO

[n.pettik]

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


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

Some statements? Lets write ‘when SQL statements are executed’ or sort of.

> 
> Part of #2618
> 
> @TarantoolBot document
> Title: SQL function last_insert_id() and IPROTO key last_insert_id.
> Function last_insert_id() returns first primary key value
> autogenerated in last INSERT/REPLACE statement in current
> session.

Mention that it is set only for AUTOINCREMENT primary key.
In other cases it will be 0.
Moreover, if you copy this feature from MySQL, you can take
part of its description: it is quite good documented there:
https://dev.mysql.com/doc/refman/8.0/en/mysql-insert-id.html <https://dev.mysql.com/doc/refman/8.0/en/mysql-insert-id.html>

The only doubt I have concerning this patch - last_insert_id()
returns FIRST autogenerated id for last stmt, which in turn
seems quite misleading (I expected that it returns LAST id of LAST stmt).
I known that MySQL uses exactly this variant, but should we follow this way?

> User can have more than one session and this
> function will work properly for each one of them. Return
> value of function is undetermined when more than one
> INSERT/REPLACE statements executed asynchronously.
> IPROTO key last_insert_id is a metadata returned through
> IPROTO after such statements as INSERT, REPLACE, UPDATE
> etc. Value of this key is equal to value returned by
> function last_insert_id() executed after the statement.
> Example:
> CREATE TABLE test (id INTEGER PRIMARY KEY AUTOINCREMENT, a INTEGER);
> INSERT INTO test VALUES (NULL, 1);
> SELECT last_insert_id();
> 
> diff --git a/src/box/session.cc b/src/box/session.cc
> index 64714cd..2b8dab9 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->sql_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..6ee22bc 100644
> --- a/src/box/session.h
> +++ b/src/box/session.h
> @@ -92,6 +92,11 @@ union session_meta {
> struct session {
> 	/** Session id. */
> 	uint64_t id;
> +	/**
> +	 * First primary key autogenerated in last INSERT/REPLACE
> +	 * statement in which primary key was generated.
> +	 */

If AUTOINCEMENT is specified, otherwise it is 0.

> +	int64_t sql_last_insert_id;
> 	/** SQL Tarantool Default storage engine. */
> 	uint8_t sql_default_engine;
> 	/** SQL Connection flag for current user session */
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 45056a7..2607cc3 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -38,6 +38,7 @@
> #include "vdbeInt.h"
> #include "version.h"
> #include "coll.h"
> +#include "box/session.h"
> #include <unicode/ustring.h>
> #include <unicode/ucasemap.h>
> #include <unicode/ucnv.h>
> @@ -601,6 +602,23 @@ changes(sqlite3_context * context, int NotUsed, sqlite3_value ** NotUsed2)
> }
> 
> /*
> + * Return first primary key autogenerated in last INSERT/REPLACE
> + * statement in which primary key was generated 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)

Nitpick: use ’struct’ prefixes.


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

[tarantool-patches] Re: [PATCH v3 1/2] sql: move autoincrement in vdbe

[n.pettik]

> This patch expands changes made in issue #2981. Now NULL
> in primary key always replaced by generated value inside
> of vdbe.

Nitpicking: during VDBE execution.

> It allows us to get last_insert_id with easy.

Nitpicking: with ease.

Actually I can’t understand purpose of this patch.
These manipulations with NULL are quite tricky.
Could you please explain in details what the bug was
about and how you fix it. Thx.

> --- a/test/sql/gh-2981-check-autoinc.test.lua
> +++ b/test/sql/gh-2981-check-autoinc.test.lua
> @@ -7,6 +7,7 @@ box.cfg{}
> box.sql.execute("CREATE TABLE t1 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
> box.sql.execute("CREATE TABLE t2 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19 AND s1 <> 25));");
> box.sql.execute("CREATE TABLE t3 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 < 10));");
> +box.sql.execute("CREATE TABLE t4 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
> 
> box.sql.execute("insert into t1 values (18, null);")
> box.sql.execute("insert into t1(s2) values (null);")
> @@ -19,7 +20,10 @@ box.sql.execute("insert into t2(s2) values (null);")
> box.sql.execute("insert into t3 values (9, null)")
> box.sql.execute("insert into t3(s2) values (null)")
> 
> +box.sql.execute("insert into t4 values (18, null);")
> +box.sql.execute("insert into t4 values (null, null);”)

I have doubts concerning this test. Firstly, AFAIK sequence
doesn’t guarantee that new values == prev_value + 1.
Thus, basically this test can fail. Moreover, why next
INSERT(NULL, NULL) doesn’t fail? For me it seems counterintuitive.
I mean I understand that sequence generator increased but insertion
failed and so forth. But does user expect such behaviour?
For instance, in MySQL this doesn’t result in error at all:

CREATE TABLE t4 (s1 int PRIMARY KEY AUTO_INCREMENT, s2 INTEGER, CHECK (s1 <> 19)) \\

insert into t4 values (18, null) \\
insert into t4 values (null, null) \\
insert into t4 values (null, null) \\
select * from t4\\

  	mysql version
1	5.7.12-log

  	s1	s2
1	18	NULL
2	19	NULL
3	20	NULL