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