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

[Imeev Mergen <imeevma@tarantool.org>]

Hi! Thank you for the review!


On 08/06/2018 06:24 PM, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch! See 11 comments below.
>
> On 03/08/2018 13:28, imeevma@tarantool.org wrote:
>> After this patch client will get last_insert_id
>> as additional metadata when he executes some
>> queries.
>>
>> Part of #2618.
>>
>> @TarantoolBot document
>
> 1. Please, describe in the same request new iproto key
> you have introduced.
Done.
>
>> Title: SQL function last_insert_id.
>> Function last_insert_id returns first autogenerated ID in
>> last INSERT/REPLACE statement in current session. Return
>> value of function is undetermined when more than one
>> INSERT/REPLACE statements executed asynchronously.
>> Example:
>> CREATE TABLE test (id INTEGER PRIMARY KEY AUTOINCREMENT, a INTEGER);
>> INSERT INTO test VALUES (NULL, 1);
>> SELECT last_insert_id();
>> ---
>> 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             |  11 ++-
>>   src/box/execute.h             |   1 +
>>   src/box/lua/net_box.c         |  12 ++-
>>   src/box/sequence.c            |  17 ++++
>>   src/box/sequence.h            |   9 +++
>>   src/box/session.cc            |   1 +
>>   src/box/session.h             |   2 +
>>   src/box/sql.c                 |   1 +
>>   src/box/sql/func.c            |  18 +++++
>>   src/box/sql/vdbe.c            |  33 ++++++++
>>   test/sql-tap/insert3.test.lua |  27 ++++++-
>>   test/sql/errinj.result        |   3 +-
>>   test/sql/iproto.result        | 180 
>> ++++++++++++++++++++++++++++++++++--------
>>   test/sql/iproto.test.lua      |  19 +++++
>>   14 files changed, 296 insertions(+), 38 deletions(-)
>>
>> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
>> index 308c9c7..0e24b47 100644
>> --- a/src/box/lua/net_box.c
>> +++ b/src/box/lua/net_box.c
>> @@ -667,16 +667,22 @@ 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;
>
> 2. Now the key is used and its (void) guard can be removed,
> it is not?
Fixed.
>
>> +    int64_t last_insert_id = mp_typeof(**data) == MP_UINT ?
>> +                 (int64_t) mp_decode_uint(data) :
>> +                 mp_decode_int(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..cefb3db 100644
>> --- a/src/box/sequence.c
>> +++ b/src/box/sequence.c
>> @@ -340,3 +340,20 @@ 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);
>> +    /*
>> +     * Number 1 is often used as start value of sequences in
>> +     * general. We are goint to use it as default value.
>> +     */
>> +    if (pos == light_sequence_end)
>> +        return 1;
>
> 3. The same comment as in the previous review. 1 is not mandatory start
> value and can be different in a common case, as well as 0, -1, 2 and any
> other constant. Use sequence_def.start.
Function was removed as it isn't necessary anymore.
>
> 4. Please, when sending a new patch version, explicitly answer my
> comments either in a new version email or in a response to my email. It
> is hard to iterate through two emails (with the comments and with your
> patch) at the same time.
Ok, understood.
>
>> +    struct sequence_data data = 
>> light_sequence_get(&sequence_data_index,
>> +                               pos);
>> +    return data.value;
>> +}
>> diff --git a/src/box/sql.c b/src/box/sql.c
>> index 2b93c3d..d9752f3 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"
>
> 5. Why?
Fixed.
>
>>   #include "index_def.h"
>>   #include "tuple.h"
>>   #include "fiber.h"
>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>> index 45056a7..eca69b7 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 "src/box/session.h"
>
> 6. For box/ dir we do not use src/ prefix.
> Please, use 'box/session.h'.
Fixed.
>
>>   #include <unicode/ustring.h>
>>   #include <unicode/ucasemap.h>
>>   #include <unicode/ucnv.h>
>> @@ -601,6 +602,22 @@ changes(sqlite3_context * context, int NotUsed, 
>> sqlite3_value ** NotUsed2)
>>   }
>>     /*
>> + * Implementation of the last_insert_id() function. Returns first
>
> 7. As I said earlier, please, do not describe the function in a
> comment like it is an object. Use imperative. I see that other
> functions in the file use the same style, but do not follow it -
> this style is sqlite's obsolete one.
Fixed.
>
>> + * autogenerated ID in last INSERT/REPLACE 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.
>>    */
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index ca89908..38cd501 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -567,6 +567,30 @@ out2Prerelease(Vdbe *p, VdbeOp *pOp)
>>       }
>>   }
>>   +/**
>> + * Check that new ID is autogenerated in current query. If it is
>> + * this function sets sql_last_insert_id of current session as
>> + * first autogenerated ID in last INSERT/REPLACE query executed
>> + * in current session.
>> + *
>> + * @param space space to get sequence from.
>> + * @retval true new id autogenerated.
>> + * @retval false no new id autogenerated.
>> + */
>> +static inline bool
>> +check_last_insert_id(struct space *space)
>> +{
>> +    int64_t new_id = 0;
>> +    struct session *session = current_session();
>> +    struct sequence *sequence = space->sequence;
>> +    if (sequence != NULL)
>> +        new_id = sequence_get_value(sequence);
>
> 8. If sequence == NULL, this function should not do anything,
> it is not? I think, it is simpler to just inline it and
> remove some of checks. See the comment 10.
Unnecessary as way last_insert_id() works changed.
>
>> +    if (session->sql_last_insert_id == new_id)
>> +        return false;
>> +    session->sql_last_insert_id = new_id;
>> +    return true;
>> +}
>> +
>>   /*
>>    * Execute as much of a VDBE program as we can.
>>    * This is the core of sqlite3_step().
>> @@ -599,6 +623,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 query.
>> +     */
>> +    bool last_insert_id_is_set = false;
>
> 9. For flags we use 'is_' prefix.
Fixed.
>
>>       /*** INSERT STACK UNION HERE ***/
>>         assert(p->magic==VDBE_MAGIC_RUN);  /* sqlite3_step() verifies 
>> this */
>> @@ -4300,6 +4329,10 @@ case OP_IdxInsert: {        /* in2 */
>>                   rc = tarantoolSqlite3Replace(pBtCur->space,
>>                                    pIn2->z,
>>                                    pIn2->z + pIn2->n);
>> +            if (rc == 0 && !last_insert_id_is_set) {
>> +                last_insert_id_is_set =
>> +                    check_last_insert_id(pBtCur->space);
>
> 10. I propose to rewrite it as
>
> if (!last_insert_id_is_set && rc == 0 && space->sequence != NULL) {
>     last_insert_id_is_set = true;
>     user_session->sql_last_insert_id =
>         sequence_get_value(space->sequence);
> }
>
> And remove check_last_insert_id.
Partially fixed.
>
>> +            }
>>           } else if (pBtCur->curFlags & BTCF_TEphemCursor) {
>>               rc = tarantoolSqlite3EphemeralInsert(pBtCur->space,
>>                                    pIn2->z,> @@ -580,6 +612,90 @@ 
>> 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.
>
> 11. 'In last insert' of what or whither?
Fixed.

commit 2b292311e61f3c25a4c776bd75e3e6d241ff66a1
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Tue Jul 31 16:43:38 2018 +0300

     sql: return last_insert_id via IPROTO

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

     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. 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 queries as INSERT, REPLACE, UPDATE etc.
     Value of this key is equal to value returned by function
     last_insert_id() executed after the query.
     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/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..83ba9b8 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -667,16 +667,21 @@ 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_typeof(**data) == MP_UINT ?
+                 (int64_t) mp_decode_uint(data) :
+                 mp_decode_int(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/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..3d4049c 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -92,6 +92,8 @@ union session_meta {
  struct session {
      /** Session id. */
      uint64_t id;
+    /** First PK autogenerated in last INSERT/REPLACE query */
+    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..164bb6e 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,22 @@ changes(sqlite3_context * context, int NotUsed, 
sqlite3_value ** NotUsed2)
  }

  /*
+ * Return first PK autogenerated in last INSERT/REPLACE in which
+ * PK 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 +1864,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/insert.c b/src/box/sql/insert.c
index 13cb61e..8b5b0df 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 dc5146f..dceb8c0 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -600,6 +600,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 query.
+     */
+    bool is_last_insert_id_set = false;
      /*** INSERT STACK UNION HERE ***/

      assert(p->magic==VDBE_MAGIC_RUN);  /* sqlite3_step() verifies this */
@@ -1147,6 +1152,10 @@ case OP_NextAutoincValue: {
      assert(pOp->p1 > 0);
      assert(pOp->p2 > 0);

+    pIn1 = &p->aMem[pOp->p2];
+    if ((pIn1->flags & MEM_Null) == 0)
+        break;
+
      struct space *space = space_by_id(pOp->p1);
      if (space == NULL) {
          rc = SQL_TARANTOOL_ERROR;
@@ -1164,6 +1173,13 @@ case OP_NextAutoincValue: {
      pOut->flags = MEM_Int;
      pOut->u.i = value;

+    /* Set sql_last_insert_id of current session. */
+    if (!is_last_insert_id_set) {
+        struct session *session = current_session();
+        session->sql_last_insert_id = value;
+        is_last_insert_id_set = true;
+    }
+
      break;
  }

@@ -3745,7 +3761,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;
+        if (pOut->flags & MEM_Int) pOut->flags = MEM_Null;
          /* Flag is set and register is NULL -> do nothing  */
      } else {
          assert(memIsValid(pIn1));
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/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")
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index af474bc..5fdf49b 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)', nil, nil)
+---
+- last_insert_id: 0
+  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: 3
+  rowcount: 1
+...
+cn:execute('update test set a = 11 where id == 1', nil, nil)
+---
+- last_insert_id: 3
+  rowcount: 1
+...
+cn:execute('update test set a = 12 where id == 2', nil, nil)
+---
+- last_insert_id: 3
+  rowcount: 1
+...
+cn:execute('update test set a = 13 where id == 3', nil, nil)
+---
+- last_insert_id: 3
+  rowcount: 1
+...
+cn:execute('replace into test values (4, 44), (null, 100)', nil, nil)
+---
+- last_insert_id: 18
+  rowcount: 2
+...
+cn:execute('insert into test values (null, 2), (null, 3)', nil, nil)
+---
+- last_insert_id: 19
+  rowcount: 2
+...
+cn:execute('select * from test', nil, nil)
+---
+- 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..db1ade5 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)', 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('update test set a = 11 where id == 1', nil, nil)
+cn:execute('update test set a = 12 where id == 2', nil, nil)
+cn:execute('update test set a = 13 where id == 3', nil, nil)
+cn:execute('replace into test values (4, 44), (null, 100)', nil, nil)
+cn:execute('insert into test values (null, 2), (null, 3)', nil, nil)
+cn:execute('select * from test', nil, nil)
+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