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 735B22D08B for ; Sat, 27 Oct 2018 07:18:13 -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 Zw2WaqL1VbXI for ; Sat, 27 Oct 2018 07:18:13 -0400 (EDT) Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 08A502C7D9 for ; Sat, 27 Oct 2018 07:18:12 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v7 1/2] sql: return all generated ids via IPROTO Date: Sat, 27 Oct 2018 14:18:10 +0300 Message-Id: In-Reply-To: References: 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: korablev@tarantool.org, tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org 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 --- src/box/execute.c | 28 ++++++++++++++- src/box/execute.h | 1 + src/box/lua/net_box.c | 29 ++++++++++++--- src/box/sequence.c | 7 ++++ src/box/sequence.h | 13 +++++++ src/box/sql/vdbe.c | 27 ++++++++++++-- src/box/sql/vdbe.h | 12 ++++++- src/box/sql/vdbeInt.h | 5 +++ src/box/sql/vdbeaux.c | 21 ++++++----- src/box/txn.h | 27 ++++++++++++++ test/sql/iproto.result | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ test/sql/iproto.test.lua | 24 +++++++++++++ 12 files changed, 268 insertions(+), 18 deletions(-) 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..5a137e0 100644 --- a/src/box/lua/net_box.c +++ b/src/box/lua/net_box.c @@ -668,16 +668,35 @@ 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); + for (uint32_t j = 0; j < count; ++j) { + int64_t id; + mp_read_int64(data, &id); + luaL_pushint64(L, id); + lua_rawseti(L, -2, j + 1); + } + lua_setfield(L, -2, "generated_ids"); + } } static int diff --git a/src/box/sequence.c b/src/box/sequence.c index 35b7605..9a87a56 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(), value) != 0) + return -1; return 0; overflow: if (!def->cycle) { diff --git a/src/box/sequence.h b/src/box/sequence.h index 0f6c8da..7180ead 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,18 @@ int access_check_sequence(struct sequence *seq); /** + * Append a new sequence value to Vdbe. List of automatically + * generated identifiers is returned as metadata of SQL response. + * + * @param Vdbe VDBE to save id in. + * @param id ID to save in VDBE. + * @retval 0 Success. + * @retval -1 Error. + */ +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..2571492 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(). @@ -2995,8 +3015,9 @@ case OP_TransactionBegin: { * If there is no active transaction, raise an error. */ case OP_TransactionCommit: { - if (box_txn()) { - if (box_txn_commit() != 0) { + struct txn *txn = in_txn(); + if (txn != NULL) { + if (txn_commit(txn) != 0) { rc = SQL_TARANTOOL_ERROR; goto abort_due_to_error; } @@ -3040,7 +3061,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..ed7fb2f 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 v Vdbe with which associate this transaction. * @retval NULL on OOM, new psql_txn struct on success. **/ struct sql_txn * -sql_alloc_txn(); +sql_alloc_txn(struct Vdbe *v); /** * Prepare given VDBE to execution: initialize structs connected @@ -197,6 +198,15 @@ sql_alloc_txn(); int sql_vdbe_prepare(struct Vdbe *vdbe); +/** + * Return pointer to list of generated ids. + * + * @param vdbe VDBE to get list of generated ids from. + * @retval List of generated ids. + */ +struct stailq * +vdbe_generated_ids(struct Vdbe *vdbe); + int sqlite3VdbeAddOp0(Vdbe *, int); int sqlite3VdbeAddOp1(Vdbe *, int, int); int sqlite3VdbeAddOp2(Vdbe *, int, int, int); diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index ce97f49..7a7d3de 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -367,6 +367,11 @@ struct Vdbe { struct sql_txn *psql_txn; /** The auto-commit flag. */ bool auto_commit; + /** + * List of ids generated in current VDBE. It is returned + * as metadata of SQL response. + */ + struct stailq generated_ids; /* When allocating a new Vdbe object, all of the fields below should be * initialized to zero or NULL diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 6e42d05..ae73f25 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -57,6 +57,7 @@ sqlite3VdbeCreate(Parse * pParse) return 0; memset(p, 0, sizeof(Vdbe)); p->db = db; + stailq_create(&p->generated_ids); if (db->pVdbe) { db->pVdbe->pPrev = p; } @@ -75,7 +76,7 @@ sqlite3VdbeCreate(Parse * pParse) } struct sql_txn * -sql_alloc_txn() +sql_alloc_txn(struct Vdbe *v) { struct sql_txn *txn = region_alloc_object(&fiber()->gc, struct sql_txn); @@ -84,6 +85,8 @@ sql_alloc_txn() "struct sql_txn"); return NULL; } + txn->vdbe = v; + v->psql_txn = txn; txn->pSavepoint = NULL; txn->fk_deferred_count = 0; return txn; @@ -103,11 +106,12 @@ 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; + } else { + vdbe->psql_txn = txn->psql_txn; } - vdbe->psql_txn = txn->psql_txn; } else { vdbe->psql_txn = NULL; } @@ -2261,12 +2265,11 @@ 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; } - p->psql_txn = ptxn->psql_txn; return 0; } @@ -2414,9 +2417,9 @@ sqlite3VdbeHalt(Vdbe * p) * key constraints to hold up the transaction. This means a commit * is required. */ - rc = box_txn_commit() == - 0 ? SQLITE_OK : - SQL_TARANTOOL_ERROR; + rc = (in_txn() == NULL || + txn_commit(in_txn()) == 0) ? + SQLITE_OK : SQL_TARANTOOL_ERROR; closeCursorsAndFree(p); } if (rc == SQLITE_BUSY && !p->pDelFrame) { @@ -2780,6 +2783,8 @@ sqlite3VdbeDelete(Vdbe * p) } p->magic = VDBE_MAGIC_DEAD; p->db = 0; + if (in_txn() == NULL) + fiber_gc(); sqlite3DbFree(db, p); } diff --git a/src/box/txn.h b/src/box/txn.h index e74c14d..12e9d10 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -117,6 +117,19 @@ struct sql_txn { * VDBE to the next in the same transaction. */ uint32_t fk_deferred_count; + /** Current VDBE. */ + struct Vdbe *vdbe; +}; + +/** + * An element of list of autogenerated ids, being returned as SQL + * response metadata. + */ +struct id_entry { + /** A link in a generated id list. */ + struct stailq_entry link; + /** Generated id. */ + int64_t id; }; struct txn { @@ -337,6 +350,20 @@ txn_last_stmt(struct txn *txn) void txn_on_stop(struct trigger *trigger, void *event); +/** + * Return VDBE that is being currently executed. + * + * @retval VDBE that is being 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..af4fc12 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -580,6 +580,98 @@ cn:close() box.sql.execute('drop table test') --- ... +-- gh-2618 Return generated columns after INSERT in IPROTO. +-- Return all ids generated in current INSERT statement. +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] +... +s = box.schema.create_space('test2', {engine = engine}) +--- +... +sq = box.schema.sequence.create('test2') +--- +... +pk = s:create_index('pk', {sequence = 'test2'}) +--- +... +function push_id() s:replace{box.NULL} s:replace{box.NULL} end +--- +... +_ = box.space.TEST:on_replace(push_id) +--- +... +cn:execute('insert into test values (null, 1)') +--- +- generated_ids: + - 127 + - 1 + - 2 + rowcount: 1 +... +cn:close() +--- +... +box.sql.execute('drop table test') +--- +... +s:drop() +--- +... +sq:drop() +--- +... 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..a2caa57 100644 --- a/test/sql/iproto.test.lua +++ b/test/sql/iproto.test.lua @@ -201,6 +201,30 @@ 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') + +s = box.schema.create_space('test2', {engine = engine}) +sq = box.schema.sequence.create('test2') +pk = s:create_index('pk', {sequence = 'test2'}) +function push_id() s:replace{box.NULL} s:replace{box.NULL} end +_ = box.space.TEST:on_replace(push_id) +cn:execute('insert into test values (null, 1)') + +cn:close() +box.sql.execute('drop table test') +s:drop() +sq:drop() + box.schema.user.revoke('guest', 'read,write,execute', 'universe') space = nil -- 2.7.4