From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 8FFEF2B802 for ; Thu, 18 Oct 2018 07:29:21 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mj-v1yHZzeFM for ; Thu, 18 Oct 2018 07:29:21 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id E16CB2B7ED for ; Thu, 18 Oct 2018 07:29:20 -0400 (EDT) From: Imeev Mergen Subject: [tarantool-patches] Re: [PATCH v5 1/1] sql: return all generated ids via IPROTO References: <23421254ee60958d4583af90100d77b222eb2aad.1538153530.git.imeevma@gmail.com> <20180929043617.GD32712@chai> Message-ID: Date: Thu, 18 Oct 2018 14:29:18 +0300 MIME-Version: 1.0 In-Reply-To: <20180929043617.GD32712@chai> Content-Type: multipart/alternative; boundary="------------C116A5EA9FDA17927CD4DC6B" Content-Language: en-US Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy This is a multi-part message in MIME format. --------------C116A5EA9FDA17927CD4DC6B Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Hello! Thank you for review! New patch below. On 09/29/2018 07:36 AM, Konstantin Osipov wrote: > *imeevma@tarantool.org [18/09/28 19:53]: >> diff --git a/src/box/execute.c b/src/box/execute.c >> index 24459b4..d9fe33f 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/vdbeInt.h" > Ugh. You're including a header which ends with Int. Have you > thought what this Int means? It's not Integer. Fixed. >> >> const char *sql_type_strs[] = { >> NULL, >> @@ -639,14 +640,37 @@ err: >> assert(port_tuple->size == 0); >> if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0) >> goto err; >> + struct stailq *id_list = &((struct Vdbe *)stmt)->id_list; >> + uint64_t id_count = 0; > The name is very general. autoinc_id_list would be more specific. > Why not simply call it generated_ids here and everywhere? This is > what these values are called in the standard. Fixed. >> int changes = sqlite3_changes(db); >> int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) + >> mp_sizeof_uint(changes); >> + if (stailq_empty(id_list) == 0) { > What is the point of this == 0? Why are you comparing with an > integer here, not boolean? Fixed. >> + struct id_entry *id_entry; >> + stailq_foreach_entry(id_entry, id_list, link) { >> + size += id_entry->id >= 0 ? >> + mp_sizeof_uint(id_entry->id) : >> + mp_sizeof_int(id_entry->id); > How can you get a negative id? It is possible: box.sql.execute('create table a (id int primary key autoincrement)') box.schema.sequence.alter('A', {min=-10000, step=-1}) box.sql.execute('insert into a values (null)') box.sql.execute('insert into a values (null)') box.sql.execute('insert into a values (null)') box.sql.execute('insert into a values (null)') box.sql.execute('insert into a values (null)') box.sql.execute('select * from a') --- - - [-3]   - [-2]   - [-1]   - [0]   - [1] ... >> + id_count++; >> + } >> + size += mp_sizeof_uint(SQL_INFO_GENERATED_IDS) + >> + mp_sizeof_array(id_count); >> + } >> char *buf = obuf_alloc(out, size); >> if (buf == NULL) { >> diag_set(OutOfMemory, size, "obuf_alloc", "buf"); >> goto err; >> } >> + if (id_count > 0) { >> + buf = mp_encode_uint(buf, SQL_INFO_GENERATED_IDS); >> + buf = mp_encode_array(buf, id_count); >> + struct id_entry *id_entry; >> + stailq_foreach_entry(id_entry, id_list, link) { >> + buf = id_entry->id >= 0 ? >> + mp_encode_uint(buf, id_entry->id) : >> + mp_encode_int(buf, id_entry->id); >> + } >> + } >> buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT); >> buf = mp_encode_uint(buf, changes); >> } >> 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, >> sql_info_key_MAX, >> }; >> >> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c >> index a928a4c..5d2ac78 100644 >> --- a/src/box/lua/net_box.c >> +++ b/src/box/lua/net_box.c >> @@ -671,11 +671,28 @@ netbox_decode_sql_info(struct lua_State *L, const char **data) >> /* Only SQL_INFO_ROW_COUNT is available. */ >> assert(map_size == 1); >> (void) map_size; >> + lua_newtable(L); >> uint32_t key = mp_decode_uint(data); >> + /* >> + * If SQL_INFO_GENERATED_IDS in data then it should be >> + * just before SQL_INFO_ROW_COUNT. >> + */ >> + if (key == SQL_INFO_GENERATED_IDS) { >> + uint64_t count = mp_decode_array(data); >> + assert (count > 0); >> + lua_createtable(L, 0, count); >> + lua_setfield(L, -2, "generated_ids"); >> + lua_getfield(L, -1, "generated_ids"); >> + for (uint32_t j = 0; j < count; ++j) { >> + int64_t value = mp_decode_uint(data); >> + lua_pushinteger(L, value); >> + lua_rawseti(L, -2, j + 1); >> + } >> + lua_pop(L, 1); >> + 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); >> lua_pushinteger(L, row_count); >> lua_setfield(L, -2, "rowcount"); >> } >> diff --git a/src/box/sequence.c b/src/box/sequence.c >> index 35b7605..ed6b2da 100644 >> --- a/src/box/sequence.c >> +++ b/src/box/sequence.c >> @@ -38,6 +38,7 @@ >> #include >> #include >> >> +#include "txn.h" >> #include "diag.h" >> #include "error.h" >> #include "errcode.h" >> @@ -178,6 +179,30 @@ sequence_update(struct sequence *seq, int64_t value) >> return 0; >> } >> >> +/** >> + * Save generated id in VDBE. >> + * >> + * @param value ID to save in VDBE. >> + * @retval 0 Success. >> + * @retval -1 Error. >> + */ >> +static inline int >> +add_new_id_in_vdbe(int64_t value) > subject-verb-object Fixed. >> +{ >> + struct txn *txn = in_txn(); >> + if (txn == NULL || txn->psql_txn == NULL) >> + return 0; >> + assert(txn->psql_txn->vdbe != NULL); >> + struct id_entry *id_entry = malloc(sizeof(*id_entry)); > Why malloc? Fixed. To save ids in region fiber_gc() was moved from txn_commit() to sqlite3VdbeDelete(). >> + if (id_entry == NULL) { >> + diag_set(OutOfMemory, sizeof(*id_entry), "malloc", "id_entry"); >> + return -1; >> + } >> + id_entry->id = value; >> + stailq_add_tail_entry(txn->psql_txn->id_list, id_entry, link); >> + return 0; >> +} >> + >> int >> sequence_next(struct sequence *seq, int64_t *result) >> { >> @@ -194,6 +219,8 @@ sequence_next(struct sequence *seq, int64_t *result) >> new_data) == light_sequence_end) >> return -1; >> *result = def->start; >> + if(add_new_id_in_vdbe(*result) != 0) >> + return -1; >> return 0; >> } >> old_data = light_sequence_get(&sequence_data_index, pos); >> @@ -228,6 +255,8 @@ done: >> new_data, &old_data) == light_sequence_end) >> unreachable(); >> *result = value; >> + if(add_new_id_in_vdbe(*result) != 0) >> + return -1; > You continue to miss a space after if. Fixed. >> return 0; >> overflow: >> if (!def->cycle) { >> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c >> index 00ffb0c..cbec9a2 100644 >> --- a/src/box/sql/vdbe.c >> +++ b/src/box/sql/vdbe.c >> @@ -3040,7 +3040,7 @@ case OP_TransactionRollback: { >> */ >> case OP_TTransaction: { >> if (!box_txn()) { >> - if (box_txn_begin() != 0) { >> + if (sql_txn_begin(p) != 0) { >> rc = SQL_TARANTOOL_ERROR; >> goto abort_due_to_error; >> } >> diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h >> index d5da5d5..9b94183 100644 >> --- a/src/box/sql/vdbe.h >> +++ b/src/box/sql/vdbe.h >> @@ -179,10 +179,11 @@ Vdbe *sqlite3VdbeCreate(Parse *); >> * Allocate and initialize SQL-specific struct which completes >> * original Tarantool's txn struct using region allocator. >> * >> + * @param vdbe VDBE that is being prepared or executed. >> * @retval NULL on OOM, new psql_txn struct on success. >> **/ >> struct sql_txn * >> -sql_alloc_txn(); >> +sql_alloc_txn(struct Vdbe *vdbe); >> >> /** >> * Prepare given VDBE to execution: initialize structs connected >> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h >> index ce97f49..ca1794c 100644 >> --- a/src/box/sql/vdbeInt.h >> +++ b/src/box/sql/vdbeInt.h >> @@ -368,6 +368,9 @@ struct Vdbe { >> /** The auto-commit flag. */ >> bool auto_commit; >> >> + /** List of id generated in current VDBE. */ >> + struct stailq id_list; >> + >> /* 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 54edf1b..98ddef7 100644 >> --- a/src/box/sql/vdbeaux.c >> +++ b/src/box/sql/vdbeaux.c >> @@ -57,6 +57,8 @@ sqlite3VdbeCreate(Parse * pParse) >> return 0; >> memset(p, 0, sizeof(Vdbe)); >> p->db = db; >> + /* Init list of generated ids. */ >> + stailq_create(&p->id_list); >> if (db->pVdbe) { >> db->pVdbe->pPrev = p; >> } >> @@ -75,7 +77,7 @@ sqlite3VdbeCreate(Parse * pParse) >> } >> >> struct sql_txn * >> -sql_alloc_txn() >> +sql_alloc_txn(struct Vdbe *vdbe) >> { >> struct sql_txn *txn = region_alloc_object(&fiber()->gc, >> struct sql_txn); >> @@ -86,6 +88,8 @@ sql_alloc_txn() >> } >> txn->pSavepoint = NULL; >> txn->fk_deferred_count = 0; >> + txn->vdbe = vdbe; >> + txn->id_list = &vdbe->id_list; > Why do you need both? Fixed. >> return txn; >> } >> >> @@ -103,7 +107,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe) >> * check FK violations, at least now. >> */ >> if (txn->psql_txn == NULL) { >> - txn->psql_txn = sql_alloc_txn(); >> + txn->psql_txn = sql_alloc_txn(vdbe); >> if (txn->psql_txn == NULL) >> return -1; >> } >> @@ -2261,7 +2265,7 @@ sql_txn_begin(Vdbe *p) >> struct txn *ptxn = txn_begin(false); >> if (ptxn == NULL) >> return -1; >> - ptxn->psql_txn = sql_alloc_txn(); >> + ptxn->psql_txn = sql_alloc_txn(p); >> if (ptxn->psql_txn == NULL) { >> box_txn_rollback(); >> return -1; >> @@ -2746,6 +2750,11 @@ sqlite3VdbeClearObject(sqlite3 * db, Vdbe * p) >> vdbeFreeOpArray(db, p->aOp, p->nOp); >> sqlite3DbFree(db, p->aColName); >> sqlite3DbFree(db, p->zSql); >> + while (stailq_empty(&p->id_list) == 0) { >> + struct id_entry *id_entry = >> + stailq_shift_entry(&p->id_list, struct id_entry, link); >> + free(id_entry); >> + } >> #ifdef SQLITE_ENABLE_STMT_SCANSTATUS >> { *New patch:* commit 9dd5e9833aec54c819e6dae07d62624538f76246 Author: Mergen Imeev Date:   Wed Sep 26 22:07:54 2018 +0300     sql: return all generated ids via IPROTO     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.     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"  const char *sql_type_strs[] = {      NULL, @@ -637,11 +638,26 @@ err:      } else {          keys = 1;          assert(port_tuple->size == 0); -        if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0) +        struct stailq *generated_ids = +            vdbe_generated_ids((struct Vdbe *)stmt); +        uint32_t map_size = stailq_empty(generated_ids) ? 1 : 2; +        if (iproto_reply_map_key(out, map_size, IPROTO_SQL_INFO) != 0)              goto err; +        uint64_t id_count = 0;          int changes = sqlite3_changes(db);          int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +                 mp_sizeof_uint(changes); +        if (!stailq_empty(generated_ids)) { +            struct id_entry *id_entry; +            stailq_foreach_entry(id_entry, generated_ids, link) { +                size += id_entry->id >= 0 ? +                    mp_sizeof_uint(id_entry->id) : +                    mp_sizeof_int(id_entry->id); +                id_count++; +            } +            size += mp_sizeof_uint(SQL_INFO_GENERATED_IDS) + +                mp_sizeof_array(id_count); +        }          char *buf = obuf_alloc(out, size);          if (buf == NULL) {              diag_set(OutOfMemory, size, "obuf_alloc", "buf"); @@ -649,6 +665,16 @@ err:          }          buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT);          buf = mp_encode_uint(buf, changes); +        if (!stailq_empty(generated_ids)) { +            buf = mp_encode_uint(buf, SQL_INFO_GENERATED_IDS); +            buf = mp_encode_array(buf, id_count); +            struct id_entry *id_entry; +            stailq_foreach_entry(id_entry, generated_ids, link) { +                buf = id_entry->id >= 0 ? +                      mp_encode_uint(buf, id_entry->id) : +                      mp_encode_int(buf, id_entry->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..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,      sql_info_key_MAX,  }; diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c index a928a4c..1a9b3f7 100644 --- a/src/box/lua/net_box.c +++ b/src/box/lua/net_box.c @@ -668,16 +668,37 @@ 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; +    assert(map_size == 1 || map_size == 2); +    lua_newtable(L); +    /* +     * First element in data is SQL_INFO_ROW_COUNT. +     */      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);      lua_pushinteger(L, row_count);      lua_setfield(L, -2, "rowcount"); +    /* +     * If data have two elements then second is +     * SQL_INFO_GENERATED_IDS. +     */ +    if (map_size == 2) { +        key = mp_decode_uint(data); +        assert(key == SQL_INFO_GENERATED_IDS); +        (void)key; +        uint64_t count = mp_decode_array(data); +        assert (count > 0); +        lua_createtable(L, 0, count); +        lua_setfield(L, -2, "generated_ids"); +        lua_getfield(L, -1, "generated_ids"); +        for (uint32_t j = 0; j < count; ++j) { +            int64_t id; +            mp_read_int64(data, &id); +            lua_pushinteger(L, id); +            lua_rawseti(L, -2, j + 1); +        } +        lua_pop(L, 1); +    }  }  static int diff --git a/src/box/sequence.c b/src/box/sequence.c index 35b7605..b1c68e9 100644 --- a/src/box/sequence.c +++ b/src/box/sequence.c @@ -38,6 +38,7 @@  #include  #include +#include "txn.h"  #include "diag.h"  #include "error.h"  #include "errcode.h" @@ -194,6 +195,9 @@ sequence_next(struct sequence *seq, int64_t *result)                        new_data) == light_sequence_end)              return -1;          *result = def->start; +        if (txn_vdbe() != NULL && +            vdbe_add_new_generated_id(txn_vdbe(), *result) != 0) +            return -1;          return 0;      }      old_data = light_sequence_get(&sequence_data_index, pos); @@ -228,6 +232,9 @@ done:                     new_data, &old_data) == light_sequence_end)          unreachable();      *result = value; +    if (txn_vdbe() != NULL && +        vdbe_add_new_generated_id(txn_vdbe(), *result) != 0) +        return -1;      return 0;  overflow:      if (!def->cycle) { diff --git a/src/box/sequence.h b/src/box/sequence.h index 0f6c8da..d38fb2f 100644 --- a/src/box/sequence.h +++ b/src/box/sequence.h @@ -43,6 +43,7 @@ extern "C" {  #endif /* defined(__cplusplus) */  struct iterator; +struct Vdbe;  /** Sequence metadata. */  struct sequence_def { @@ -140,6 +141,17 @@ int  access_check_sequence(struct sequence *seq);  /** + * Add new generated id in VDBE. + * + * @param Vdbe VDBE to save id in. + * @param id ID to save in VDBE. + * @retval 0 Success. + * @retval -1 Error. + */ +int +vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id); + +/**   * Create an iterator over sequence data.   *   * The iterator creates a snapshot of sequence data and walks diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7c1015c..7a3748e 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -578,6 +578,26 @@ out2Prerelease(Vdbe *p, VdbeOp *pOp)      }  } +struct stailq * +vdbe_generated_ids(struct Vdbe *vdbe) +{ +    return &vdbe->generated_ids; +} + +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"); +        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(). @@ -2996,7 +3016,7 @@ case OP_TransactionBegin: {   */  case OP_TransactionCommit: {      if (box_txn()) { -        if (box_txn_commit() != 0) { +        if (txn_commit(in_txn()) != 0) {              rc = SQL_TARANTOOL_ERROR;              goto abort_due_to_error;          } @@ -3040,7 +3060,7 @@ case OP_TransactionRollback: {   */  case OP_TTransaction: {      if (!box_txn()) { -        if (box_txn_begin() != 0) { +        if (sql_txn_begin(p) != 0) {              rc = SQL_TARANTOOL_ERROR;              goto abort_due_to_error;          } diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h index d5da5d5..8f877b3 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -197,6 +197,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); +  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..37df5e2 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -368,6 +368,9 @@ struct Vdbe {      /** The auto-commit flag. */      bool auto_commit; +    /** List of id generated in current VDBE. */ +    struct stailq generated_ids; +      /* 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..0023dc6 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -57,6 +57,8 @@ sqlite3VdbeCreate(Parse * pParse)          return 0;      memset(p, 0, sizeof(Vdbe));      p->db = db; +    /* Init list of generated ids. */ +    stailq_create(&p->generated_ids);      if (db->pVdbe) {          db->pVdbe->pPrev = p;      } @@ -106,6 +108,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe)              txn->psql_txn = sql_alloc_txn();              if (txn->psql_txn == NULL)                  return -1; +            txn->psql_txn->vdbe = vdbe;          }          vdbe->psql_txn = txn->psql_txn;      } else { @@ -2266,6 +2269,7 @@ sql_txn_begin(Vdbe *p)          box_txn_rollback();          return -1;      } +    ptxn->psql_txn->vdbe = p;      p->psql_txn = ptxn->psql_txn;      return 0;  } @@ -2414,9 +2418,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 +2784,8 @@ sqlite3VdbeDelete(Vdbe * p)      }      p->magic = VDBE_MAGIC_DEAD;      p->db = 0; +    if (in_txn() == NULL) +        fiber_gc();      sqlite3DbFree(db, p);  } diff --git a/src/box/txn.h b/src/box/txn.h index e74c14d..2acba9f 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -117,6 +117,17 @@ 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 generated ids. */ +struct id_entry +{ +    /** A link in a generated id list. */ +    struct stailq_entry link; +    /** Generated id. */ +    int64_t id;  };  struct txn { @@ -337,6 +348,20 @@ txn_last_stmt(struct txn *txn)  void  txn_on_stop(struct trigger *trigger, void *event); +/** + * Return VDBE that is currently executed. + * + * @retval VDBE that is currently executed. + * @retval NULL Either txn or ptxn_sql or vdbe is NULL; + */ +static inline struct Vdbe * +txn_vdbe() +{ +    struct txn *txn = in_txn(); +    if (txn == NULL || txn->psql_txn == NULL) +        return NULL; +    return txn->psql_txn->vdbe; +}  /**   * FFI bindings: do not throw exceptions, do not accept extra diff --git a/test/sql/iproto.result b/test/sql/iproto.result index af474bc..163fb6e 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -580,6 +580,77 @@ 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. +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)') +--- +- rowcount: 1 +... +cn:execute('insert into test values (null, 2)') +--- +- generated_ids: +  - 2 +  rowcount: 1 +... +cn:execute('update test set a = 11 where id == 1') +--- +- rowcount: 1 +... +cn:execute('insert into test values (100, 1), (null, 1), (120, 1), (null, 1)') +--- +- generated_ids: +  - 101 +  - 121 +  rowcount: 4 +... +cn:execute('insert into test values (null, 1), (null, 1), (null, 1), (null, 1), (null, 1)') +--- +- generated_ids: +  - 122 +  - 123 +  - 124 +  - 125 +  - 126 +  rowcount: 5 +... +cn:execute('select * from test') +--- +- metadata: +  - name: ID +  - name: A +  rows: +  - [1, 11] +  - [2, 2] +  - [100, 1] +  - [101, 1] +  - [120, 1] +  - [121, 1] +  - [122, 1] +  - [123, 1] +  - [124, 1] +  - [125, 1] +  - [126, 1] +... +cn:execute('create table test2 (id integer primary key, a integer)') +--- +- rowcount: 1 +... +cn:execute('drop table test2') +--- +- 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..6517cde 100644 --- a/test/sql/iproto.test.lua +++ b/test/sql/iproto.test.lua @@ -201,6 +201,21 @@ future4:wait_result()  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. +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('update test set a = 11 where id == 1') +cn:execute('insert into test values (100, 1), (null, 1), (120, 1), (null, 1)') +cn:execute('insert into test values (null, 1), (null, 1), (null, 1), (null, 1), (null, 1)') +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 --------------C116A5EA9FDA17927CD4DC6B Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

Hello! Thank you for review! New patch below.


On 09/29/2018 07:36 AM, Konstantin Osipov wrote:
* imeevma@tarantool.org <imeevma@tarantool.org> [18/09/28 19:53]:

diff --git a/src/box/execute.c b/src/box/execute.c
index 24459b4..d9fe33f 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/vdbeInt.h"

Ugh. You're including a header which ends with Int. Have you
thought what this Int means? It's not Integer.
Fixed.

      

        
 
 const char *sql_type_strs[] = {
 	NULL,
@@ -639,14 +640,37 @@ err:
 		assert(port_tuple->size == 0);
 		if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0)
 			goto err;
+		struct stailq *id_list = &((struct Vdbe *)stmt)->id_list;
+		uint64_t id_count = 0;


      

      
The name is very general. autoinc_id_list would be more specific.
Why not simply call it generated_ids here and everywhere? This is
what these values are called in the standard.

    

    Fixed.

    

      

      

        
 		int changes = sqlite3_changes(db);
 		int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +
 			   mp_sizeof_uint(changes);
+		if (stailq_empty(id_list) == 0) {


      

      
What is the point of this == 0? Why are you comparing with an
integer here, not boolean? 

    

    Fixed.

    

      

      

        
+			struct id_entry *id_entry;
+			stailq_foreach_entry(id_entry, id_list, link) {
+				size += id_entry->id >= 0 ?
+					mp_sizeof_uint(id_entry->id) :
+					mp_sizeof_int(id_entry->id);


      

      
How can you get a negative id?

    

    It is possible:

    

    box.sql.execute('create table a (id int primary key autoincrement)')

    box.schema.sequence.alter('A', {min=-10000, step=-1})

    box.sql.execute('insert into a values (null)')

    box.sql.execute('insert into a values (null)')

    box.sql.execute('insert into a values (null)')

    box.sql.execute('insert into a values (null)')

    box.sql.execute('insert into a values (null)')

    box.sql.execute('select * from a')

    ---

    - - [-3]

      - [-2]

      - [-1]

      - [0]

      - [1]

    ...

    

    

      

        
+				id_count++;
+			}
+			size += mp_sizeof_uint(SQL_INFO_GENERATED_IDS) +
+				mp_sizeof_array(id_count);
+		}


      

      

      

        
 		char *buf = obuf_alloc(out, size);
 		if (buf == NULL) {
 			diag_set(OutOfMemory, size, "obuf_alloc", "buf");
 			goto err;
 		}
+		if (id_count > 0) {
+			buf = mp_encode_uint(buf, SQL_INFO_GENERATED_IDS);
+			buf = mp_encode_array(buf, id_count);
+			struct id_entry *id_entry;
+			stailq_foreach_entry(id_entry, id_list, link) {
+				buf = id_entry->id >= 0 ?
+				      mp_encode_uint(buf, id_entry->id) :
+				      mp_encode_int(buf, id_entry->id);
+			}
+		}
 		buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT);
 		buf = mp_encode_uint(buf, changes);
 	}
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,
 	sql_info_key_MAX,
 };
 
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index a928a4c..5d2ac78 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -671,11 +671,28 @@ netbox_decode_sql_info(struct lua_State *L, const char **data)
 	/* Only SQL_INFO_ROW_COUNT is available. */
 	assert(map_size == 1);
 	(void) map_size;
+	lua_newtable(L);
 	uint32_t key = mp_decode_uint(data);
+	/*
+	 * If SQL_INFO_GENERATED_IDS in data then it should be
+	 * just before SQL_INFO_ROW_COUNT.
+	 */
+	if (key == SQL_INFO_GENERATED_IDS) {
+		uint64_t count = mp_decode_array(data);
+		assert (count > 0);
+		lua_createtable(L, 0, count);
+		lua_setfield(L, -2, "generated_ids");
+		lua_getfield(L, -1, "generated_ids");
+		for (uint32_t j = 0; j < count; ++j) {
+			int64_t value = mp_decode_uint(data);
+			lua_pushinteger(L, value);
+			lua_rawseti(L, -2, j + 1);
+		}
+		lua_pop(L, 1);
+		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);
 	lua_pushinteger(L, row_count);
 	lua_setfield(L, -2, "rowcount");
 }
diff --git a/src/box/sequence.c b/src/box/sequence.c
index 35b7605..ed6b2da 100644
--- a/src/box/sequence.c
+++ b/src/box/sequence.c
@@ -38,6 +38,7 @@
 #include <small/mempool.h>
 #include <msgpuck/msgpuck.h>
 
+#include "txn.h"
 #include "diag.h"
 #include "error.h"
 #include "errcode.h"
@@ -178,6 +179,30 @@ sequence_update(struct sequence *seq, int64_t value)
 	return 0;
 }
 
+/**
+ * Save generated id in VDBE.
+ *
+ * @param value ID to save in VDBE.
+ * @retval 0 Success.
+ * @retval -1 Error.
+ */
+static inline int
+add_new_id_in_vdbe(int64_t value)


      

      
subject-verb-object

    

    Fixed.

    

      

      

        
+{
+	struct txn *txn = in_txn();
+	if (txn == NULL || txn->psql_txn == NULL)
+		return 0;
+	assert(txn->psql_txn->vdbe != NULL);
+	struct id_entry *id_entry = malloc(sizeof(*id_entry));


      

      
Why malloc?

    

    Fixed. To save ids in region fiber_gc() was moved from

    txn_commit() to sqlite3VdbeDelete().

    

      

      

        
+	if (id_entry == NULL) {
+		diag_set(OutOfMemory, sizeof(*id_entry), "malloc", "id_entry");
+		return -1;
+	}
+	id_entry->id = value;
+	stailq_add_tail_entry(txn->psql_txn->id_list, id_entry, link);
+	return 0;
+}
+
 int
 sequence_next(struct sequence *seq, int64_t *result)
 {
@@ -194,6 +219,8 @@ sequence_next(struct sequence *seq, int64_t *result)
 					  new_data) == light_sequence_end)
 			return -1;
 		*result = def->start;
+		if(add_new_id_in_vdbe(*result) != 0)
+			return -1;
 		return 0;
 	}
 	old_data = light_sequence_get(&sequence_data_index, pos);
@@ -228,6 +255,8 @@ done:
 				   new_data, &old_data) == light_sequence_end)
 		unreachable();
 	*result = value;
+	if(add_new_id_in_vdbe(*result) != 0)
+		return -1;


      

      
You continue to miss a space after if.

    

    Fixed.

    

      

        
 	return 0;
 overflow:
 	if (!def->cycle) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 00ffb0c..cbec9a2 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3040,7 +3040,7 @@ case OP_TransactionRollback: {
  */
 case OP_TTransaction: {
 	if (!box_txn()) {
-		if (box_txn_begin() != 0) {
+		if (sql_txn_begin(p) != 0) {
 			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index d5da5d5..9b94183 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -179,10 +179,11 @@ Vdbe *sqlite3VdbeCreate(Parse *);
  * Allocate and initialize SQL-specific struct which completes
  * original Tarantool's txn struct using region allocator.
  *
+ * @param vdbe VDBE that is being prepared or executed.
  * @retval NULL on OOM, new psql_txn struct on success.
  **/
 struct sql_txn *
-sql_alloc_txn();
+sql_alloc_txn(struct Vdbe *vdbe);
 
 /**
  * Prepare given VDBE to execution: initialize structs connected
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index ce97f49..ca1794c 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -368,6 +368,9 @@ struct Vdbe {
 	/** The auto-commit flag. */
 	bool auto_commit;
 
+	/** List of id generated in current VDBE. */
+	struct stailq id_list;
+
 	/* 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 54edf1b..98ddef7 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -57,6 +57,8 @@ sqlite3VdbeCreate(Parse * pParse)
 		return 0;
 	memset(p, 0, sizeof(Vdbe));
 	p->db = db;
+	/* Init list of generated ids. */
+	stailq_create(&p->id_list);
 	if (db->pVdbe) {
 		db->pVdbe->pPrev = p;
 	}
@@ -75,7 +77,7 @@ sqlite3VdbeCreate(Parse * pParse)
 }
 
 struct sql_txn *
-sql_alloc_txn()
+sql_alloc_txn(struct Vdbe *vdbe)
 {
 	struct sql_txn *txn = region_alloc_object(&fiber()->gc,
 						  struct sql_txn);
@@ -86,6 +88,8 @@ sql_alloc_txn()
 	}
 	txn->pSavepoint = NULL;
 	txn->fk_deferred_count = 0;
+	txn->vdbe = vdbe;
+	txn->id_list = &vdbe->id_list;


      

      
Why do you need both?

    

    Fixed.

    

      

      

        
 	return txn;
 }
 
@@ -103,7 +107,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
 		 * check FK violations, at least now.
 		 */
 		if (txn->psql_txn == NULL) {
-			txn->psql_txn = sql_alloc_txn();
+			txn->psql_txn = sql_alloc_txn(vdbe);
 			if (txn->psql_txn == NULL)
 				return -1;
 		}
@@ -2261,7 +2265,7 @@ sql_txn_begin(Vdbe *p)
 	struct txn *ptxn = txn_begin(false);
 	if (ptxn == NULL)
 		return -1;
-	ptxn->psql_txn = sql_alloc_txn();
+	ptxn->psql_txn = sql_alloc_txn(p);
 	if (ptxn->psql_txn == NULL) {
 		box_txn_rollback();
 		return -1;
@@ -2746,6 +2750,11 @@ sqlite3VdbeClearObject(sqlite3 * db, Vdbe * p)
 	vdbeFreeOpArray(db, p->aOp, p->nOp);
 	sqlite3DbFree(db, p->aColName);
 	sqlite3DbFree(db, p->zSql);
+	while (stailq_empty(&p->id_list) == 0) {
+		struct id_entry *id_entry =
+			stailq_shift_entry(&p->id_list, struct id_entry, link);
+		free(id_entry);
+	}
 #ifdef SQLITE_ENABLE_STMT_SCANSTATUS
 	{


      

    

    

     New patch:

    

    commit 9dd5e9833aec54c819e6dae07d62624538f76246

    Author: Mergen Imeev <imeevma@gmail.com>

    Date:   Wed Sep 26 22:07:54 2018 +0300

    

        sql: return all generated ids via IPROTO

        

        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.

        

        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"

     

     const char *sql_type_strs[] = {

         NULL,

    @@ -637,11 +638,26 @@ err:

         } else {

             keys = 1;

             assert(port_tuple->size == 0);

    -        if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0)

    +        struct stailq *generated_ids =

    +            vdbe_generated_ids((struct Vdbe *)stmt);

    +        uint32_t map_size = stailq_empty(generated_ids) ? 1 : 2;

    +        if (iproto_reply_map_key(out, map_size, IPROTO_SQL_INFO) !=
    0)

                 goto err;

    +        uint64_t id_count = 0;

             int changes = sqlite3_changes(db);

             int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +

                    mp_sizeof_uint(changes);

    +        if (!stailq_empty(generated_ids)) {

    +            struct id_entry *id_entry;

    +            stailq_foreach_entry(id_entry, generated_ids, link) {

    +                size += id_entry->id >= 0 ?

    +                    mp_sizeof_uint(id_entry->id) :

    +                    mp_sizeof_int(id_entry->id);

    +                id_count++;

    +            }

    +            size += mp_sizeof_uint(SQL_INFO_GENERATED_IDS) +

    +                mp_sizeof_array(id_count);

    +        }

             char *buf = obuf_alloc(out, size);

             if (buf == NULL) {

                 diag_set(OutOfMemory, size, "obuf_alloc", "buf");

    @@ -649,6 +665,16 @@ err:

             }

             buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT);

             buf = mp_encode_uint(buf, changes);

    +        if (!stailq_empty(generated_ids)) {

    +            buf = mp_encode_uint(buf, SQL_INFO_GENERATED_IDS);

    +            buf = mp_encode_array(buf, id_count);

    +            struct id_entry *id_entry;

    +            stailq_foreach_entry(id_entry, generated_ids, link) {

    +                buf = id_entry->id >= 0 ?

    +                      mp_encode_uint(buf, id_entry->id) :

    +                      mp_encode_int(buf, id_entry->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..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,

         sql_info_key_MAX,

     };

     

    diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c

    index a928a4c..1a9b3f7 100644

    --- a/src/box/lua/net_box.c

    +++ b/src/box/lua/net_box.c

    @@ -668,16 +668,37 @@ 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;

    +    assert(map_size == 1 || map_size == 2);

    +    lua_newtable(L);

    +    /*

    +     * First element in data is SQL_INFO_ROW_COUNT.

    +     */

         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);

         lua_pushinteger(L, row_count);

         lua_setfield(L, -2, "rowcount");

    +    /*

    +     * If data have two elements then second is

    +     * SQL_INFO_GENERATED_IDS.

    +     */

    +    if (map_size == 2) {

    +        key = mp_decode_uint(data);

    +        assert(key == SQL_INFO_GENERATED_IDS);

    +        (void)key;

    +        uint64_t count = mp_decode_array(data);

    +        assert (count > 0);

    +        lua_createtable(L, 0, count);

    +        lua_setfield(L, -2, "generated_ids");

    +        lua_getfield(L, -1, "generated_ids");

    +        for (uint32_t j = 0; j < count; ++j) {

    +            int64_t id;

    +            mp_read_int64(data, &id);

    +            lua_pushinteger(L, id);

    +            lua_rawseti(L, -2, j + 1);

    +        }

    +        lua_pop(L, 1);

    +    }

     }

     

     static int

    diff --git a/src/box/sequence.c b/src/box/sequence.c

    index 35b7605..b1c68e9 100644

    --- a/src/box/sequence.c

    +++ b/src/box/sequence.c

    @@ -38,6 +38,7 @@

     #include <small/mempool.h>

     #include <msgpuck/msgpuck.h>

     

    +#include "txn.h"

     #include "diag.h"

     #include "error.h"

     #include "errcode.h"

    @@ -194,6 +195,9 @@ sequence_next(struct sequence *seq, int64_t
    *result)

                           new_data) == light_sequence_end)

                 return -1;

             *result = def->start;

    +        if (txn_vdbe() != NULL &&

    +            vdbe_add_new_generated_id(txn_vdbe(), *result) != 0)

    +            return -1;

             return 0;

         }

         old_data = light_sequence_get(&sequence_data_index, pos);

    @@ -228,6 +232,9 @@ done:

                        new_data, &old_data) == light_sequence_end)

             unreachable();

         *result = value;

    +    if (txn_vdbe() != NULL &&

    +        vdbe_add_new_generated_id(txn_vdbe(), *result) != 0)

    +        return -1;

         return 0;

     overflow:

         if (!def->cycle) {

    diff --git a/src/box/sequence.h b/src/box/sequence.h

    index 0f6c8da..d38fb2f 100644

    --- a/src/box/sequence.h

    +++ b/src/box/sequence.h

    @@ -43,6 +43,7 @@ extern "C" {

     #endif /* defined(__cplusplus) */

     

     struct iterator;

    +struct Vdbe;

     

     /** Sequence metadata. */

     struct sequence_def {

    @@ -140,6 +141,17 @@ int

     access_check_sequence(struct sequence *seq);

     

     /**

    + * Add new generated id in VDBE.

    + *

    + * @param Vdbe VDBE to save id in.

    + * @param id ID to save in VDBE.

    + * @retval 0 Success.

    + * @retval -1 Error.

    + */

    +int

    +vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id);

    +

    +/**

      * Create an iterator over sequence data.

      *

      * The iterator creates a snapshot of sequence data and walks

    diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c

    index 7c1015c..7a3748e 100644

    --- a/src/box/sql/vdbe.c

    +++ b/src/box/sql/vdbe.c

    @@ -578,6 +578,26 @@ out2Prerelease(Vdbe *p, VdbeOp *pOp)

         }

     }

     

    +struct stailq *

    +vdbe_generated_ids(struct Vdbe *vdbe)

    +{

    +    return &vdbe->generated_ids;

    +}

    +

    +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");

    +        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().

    @@ -2996,7 +3016,7 @@ case OP_TransactionBegin: {

      */

     case OP_TransactionCommit: {

         if (box_txn()) {

    -        if (box_txn_commit() != 0) {

    +        if (txn_commit(in_txn()) != 0) {

                 rc = SQL_TARANTOOL_ERROR;

                 goto abort_due_to_error;

             }

    @@ -3040,7 +3060,7 @@ case OP_TransactionRollback: {

      */

     case OP_TTransaction: {

         if (!box_txn()) {

    -        if (box_txn_begin() != 0) {

    +        if (sql_txn_begin(p) != 0) {

                 rc = SQL_TARANTOOL_ERROR;

                 goto abort_due_to_error;

             }

    diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h

    index d5da5d5..8f877b3 100644

    --- a/src/box/sql/vdbe.h

    +++ b/src/box/sql/vdbe.h

    @@ -197,6 +197,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);

    +

     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..37df5e2 100644

    --- a/src/box/sql/vdbeInt.h

    +++ b/src/box/sql/vdbeInt.h

    @@ -368,6 +368,9 @@ struct Vdbe {

         /** The auto-commit flag. */

         bool auto_commit;

     

    +    /** List of id generated in current VDBE. */

    +    struct stailq generated_ids;

    +

         /* 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..0023dc6 100644

    --- a/src/box/sql/vdbeaux.c

    +++ b/src/box/sql/vdbeaux.c

    @@ -57,6 +57,8 @@ sqlite3VdbeCreate(Parse * pParse)

             return 0;

         memset(p, 0, sizeof(Vdbe));

         p->db = db;

    +    /* Init list of generated ids. */

    +    stailq_create(&p->generated_ids);

         if (db->pVdbe) {

             db->pVdbe->pPrev = p;

         }

    @@ -106,6 +108,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe)

                 txn->psql_txn = sql_alloc_txn();

                 if (txn->psql_txn == NULL)

                     return -1;

    +            txn->psql_txn->vdbe = vdbe;

             }

             vdbe->psql_txn = txn->psql_txn;

         } else {

    @@ -2266,6 +2269,7 @@ sql_txn_begin(Vdbe *p)

             box_txn_rollback();

             return -1;

         }

    +    ptxn->psql_txn->vdbe = p;

         p->psql_txn = ptxn->psql_txn;

         return 0;

     }

    @@ -2414,9 +2418,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 +2784,8 @@ sqlite3VdbeDelete(Vdbe * p)

         }

         p->magic = VDBE_MAGIC_DEAD;

         p->db = 0;

    +    if (in_txn() == NULL)

    +        fiber_gc();

         sqlite3DbFree(db, p);

     }

     

    diff --git a/src/box/txn.h b/src/box/txn.h

    index e74c14d..2acba9f 100644

    --- a/src/box/txn.h

    +++ b/src/box/txn.h

    @@ -117,6 +117,17 @@ 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 generated ids. */

    +struct id_entry

    +{

    +    /** A link in a generated id list. */

    +    struct stailq_entry link;

    +    /** Generated id. */

    +    int64_t id;

     };

     

     struct txn {

    @@ -337,6 +348,20 @@ txn_last_stmt(struct txn *txn)

     void

     txn_on_stop(struct trigger *trigger, void *event);

     

    +/**

    + * Return VDBE that is currently executed.

    + *

    + * @retval VDBE that is currently executed.

    + * @retval NULL Either txn or ptxn_sql or vdbe is NULL;

    + */

    +static inline struct Vdbe *

    +txn_vdbe()

    +{

    +    struct txn *txn = in_txn();

    +    if (txn == NULL || txn->psql_txn == NULL)

    +        return NULL;

    +    return txn->psql_txn->vdbe;

    +}

     

     /**

      * FFI bindings: do not throw exceptions, do not accept extra

    diff --git a/test/sql/iproto.result b/test/sql/iproto.result

    index af474bc..163fb6e 100644

    --- a/test/sql/iproto.result

    +++ b/test/sql/iproto.result

    @@ -580,6 +580,77 @@ 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.

    +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)')

    +---

    +- rowcount: 1

    +...

    +cn:execute('insert into test values (null, 2)')

    +---

    +- generated_ids:

    +  - 2

    +  rowcount: 1

    +...

    +cn:execute('update test set a = 11 where id == 1')

    +---

    +- rowcount: 1

    +...

    +cn:execute('insert into test values (100, 1), (null, 1), (120, 1),
    (null, 1)')

    +---

    +- generated_ids:

    +  - 101

    +  - 121

    +  rowcount: 4

    +...

    +cn:execute('insert into test values (null, 1), (null, 1), (null,
    1), (null, 1), (null, 1)')

    +---

    +- generated_ids:

    +  - 122

    +  - 123

    +  - 124

    +  - 125

    +  - 126

    +  rowcount: 5

    +...

    +cn:execute('select * from test')

    +---

    +- metadata:

    +  - name: ID

    +  - name: A

    +  rows:

    +  - [1, 11]

    +  - [2, 2]

    +  - [100, 1]

    +  - [101, 1]

    +  - [120, 1]

    +  - [121, 1]

    +  - [122, 1]

    +  - [123, 1]

    +  - [124, 1]

    +  - [125, 1]

    +  - [126, 1]

    +...

    +cn:execute('create table test2 (id integer primary key, a
    integer)')

    +---

    +- rowcount: 1

    +...

    +cn:execute('drop table test2')

    +---

    +- 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..6517cde 100644

    --- a/test/sql/iproto.test.lua

    +++ b/test/sql/iproto.test.lua

    @@ -201,6 +201,21 @@ future4:wait_result()

     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.

    +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('update test set a = 11 where id == 1')

    +cn:execute('insert into test values (100, 1), (null, 1), (120, 1),
    (null, 1)')

    +cn:execute('insert into test values (null, 1), (null, 1), (null,
    1), (null, 1), (null, 1)')

    +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

     

    

  


--------------C116A5EA9FDA17927CD4DC6B--