* [tarantool-patches] [PATCH v2 0/2] sql: remove struct Enc
@ 2018-08-27 11:11 Kirill Shcherbatov
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 1/2] box: export mpstream methods to core Kirill Shcherbatov
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Kirill Shcherbatov @ 2018-08-27 11:11 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov
Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3545-remove-enc-struct
Issue: https://github.com/tarantool/tarantool/issues/3545
We shouldn't use legacy Enc structure in SQL as we have
featured class mpstream in tarantool core. With this path
all allocations temporally commited with mpstream initialized
on region and then duplicated in dynamic persistent memory via
sqlite3DbMallocRaw.
As a first step, we've moved mpstream API in core lib and reworked
all legacy interfaces.
Changes in v2:
- rebased to master, merged with new func
sql_index_update_table_name.
- reworked legacy code with new mpstream library API
Kirill Shcherbatov (2):
box: export mpstream methods to core
sql: remove struct Enc
src/CMakeLists.txt | 1 +
src/box/lua/call.c | 11 +-
src/box/lua/misc.cc | 1 +
src/box/lua/net_box.c | 127 ++++++------
src/box/lua/tuple.c | 23 +--
src/box/sql.c | 473 +++++++++++++++++++++++----------------------
src/box/sql/build.c | 156 ++++++++-------
src/box/sql/tarantoolInt.h | 80 +++++---
src/box/sql/trigger.c | 62 +++---
src/lua/msgpack.c | 166 ++--------------
src/lua/msgpack.h | 102 +---------
src/mpstream.c | 205 ++++++++++++++++++++
src/mpstream.h | 124 ++++++++++++
13 files changed, 845 insertions(+), 686 deletions(-)
create mode 100644 src/mpstream.c
create mode 100644 src/mpstream.h
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] [PATCH v2 1/2] box: export mpstream methods to core
2018-08-27 11:11 [tarantool-patches] [PATCH v2 0/2] sql: remove struct Enc Kirill Shcherbatov
@ 2018-08-27 11:11 ` Kirill Shcherbatov
2018-08-28 1:43 ` [tarantool-patches] " Vladislav Shpilevoy
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 2/2] sql: remove struct Enc Kirill Shcherbatov
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Kirill Shcherbatov @ 2018-08-27 11:11 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov
As we going to use mpstream not only in LUA, let's move this
API in tarantool core.
Part of #3545.
---
src/CMakeLists.txt | 1 +
src/box/lua/call.c | 11 +--
src/box/lua/misc.cc | 1 +
src/box/lua/net_box.c | 127 +++++++++++++++----------------
src/box/lua/tuple.c | 23 +++---
src/lua/msgpack.c | 166 ++++------------------------------------
src/lua/msgpack.h | 102 +------------------------
src/mpstream.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/mpstream.h | 124 ++++++++++++++++++++++++++++++
9 files changed, 429 insertions(+), 331 deletions(-)
create mode 100644 src/mpstream.c
create mode 100644 src/mpstream.h
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 7c8b5cc..0407337 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -97,6 +97,7 @@ set (core_sources
http_parser.c
coll.c
coll_def.c
+ mpstream.c
)
if (TARGET_OS_NETBSD)
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index d20cbbb..1f20426 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -42,6 +42,7 @@
#include "small/obuf.h"
#include "lua_sql.h"
#include "trivia/util.h"
+#include "mpstream.h"
/**
* A helper to find a Lua function by name and put it
@@ -175,7 +176,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
* ..., { scalar }, ...`
*/
lua_pushvalue(L, i);
- luamp_encode_array(cfg, stream, 1);
+ mpstream_encode_array(stream, 1);
luamp_encode_r(L, cfg, stream, &field, 0);
lua_pop(L, 1);
} else {
@@ -202,7 +203,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
* `return scalar`
* `return map`
*/
- luamp_encode_array(cfg, stream, 1);
+ mpstream_encode_array(stream, 1);
assert(lua_gettop(L) == 1);
luamp_encode_r(L, cfg, stream, &root, 0);
return 1;
@@ -211,7 +212,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
assert(root.type == MP_ARRAY);
if (root.size == 0) {
/* `return {}` => `{ box.tuple() }` */
- luamp_encode_array(cfg, stream, 0);
+ mpstream_encode_array(stream, 0);
return 1;
}
@@ -230,7 +231,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
* `return { scalar, ... } =>
* box.tuple.new(scalar, ...)`
*/
- luamp_encode_array(cfg, stream, root.size);
+ mpstream_encode_array(stream, root.size);
/*
* Encode the first field of tuple using
* existing information from luaL_tofield
@@ -250,7 +251,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
* `return { tuple/array, ..., scalar, ... } =>
* { tuple/array, ..., { scalar }, ... }`
*/
- luamp_encode_array(cfg, stream, 1);
+ mpstream_encode_array(stream, 1);
luamp_encode_r(L, cfg, stream, &field, 0);
} else {
/* `return { tuple/array, ..., tuple/array, ... }` */
diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index bc76065..8bd33ae 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -38,6 +38,7 @@
#include "box/box.h"
#include "box/port.h"
#include "box/lua/tuple.h"
+#include "mpstream.h"
/** {{{ Miscellaneous utils **/
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 308c9c7..a928a4c 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -47,6 +47,7 @@
#include "coio.h"
#include "box/errcode.h"
#include "lua/fiber.h"
+#include "mpstream.h"
#define cfg luaL_msgpack_default
@@ -68,13 +69,13 @@ netbox_prepare_request(lua_State *L, struct mpstream *stream, uint32_t r_type)
mpstream_advance(stream, fixheader_size);
/* encode header */
- luamp_encode_map(cfg, stream, 2);
+ mpstream_encode_map(stream, 2);
- luamp_encode_uint(cfg, stream, IPROTO_SYNC);
- luamp_encode_uint(cfg, stream, sync);
+ mpstream_encode_uint(stream, IPROTO_SYNC);
+ mpstream_encode_uint(stream, sync);
- luamp_encode_uint(cfg, stream, IPROTO_REQUEST_TYPE);
- luamp_encode_uint(cfg, stream, r_type);
+ mpstream_encode_uint(stream, IPROTO_REQUEST_TYPE);
+ mpstream_encode_uint(stream, r_type);
/* Caller should remember how many bytes was used in ibuf */
return used;
@@ -139,16 +140,16 @@ netbox_encode_auth(lua_State *L)
return luaL_error(L, "Invalid salt");
/* Adapted from xrow_encode_auth() */
- luamp_encode_map(cfg, &stream, password != NULL ? 2 : 1);
- luamp_encode_uint(cfg, &stream, IPROTO_USER_NAME);
- luamp_encode_str(cfg, &stream, user, user_len);
+ mpstream_encode_map(&stream, password != NULL ? 2 : 1);
+ mpstream_encode_uint(&stream, IPROTO_USER_NAME);
+ mpstream_encode_strn(&stream, user, user_len);
if (password != NULL) { /* password can be omitted */
char scramble[SCRAMBLE_SIZE];
scramble_prepare(scramble, salt, password, password_len);
- luamp_encode_uint(cfg, &stream, IPROTO_TUPLE);
- luamp_encode_array(cfg, &stream, 2);
- luamp_encode_str(cfg, &stream, "chap-sha1", strlen("chap-sha1"));
- luamp_encode_str(cfg, &stream, scramble, SCRAMBLE_SIZE);
+ mpstream_encode_uint(&stream, IPROTO_TUPLE);
+ mpstream_encode_array(&stream, 2);
+ mpstream_encode_str(&stream, "chap-sha1");
+ mpstream_encode_strn(&stream, scramble, SCRAMBLE_SIZE);
}
netbox_encode_request(&stream, svp);
@@ -166,16 +167,16 @@ netbox_encode_call_impl(lua_State *L, enum iproto_type type)
struct mpstream stream;
size_t svp = netbox_prepare_request(L, &stream, type);
- luamp_encode_map(cfg, &stream, 2);
+ mpstream_encode_map(&stream, 2);
/* encode proc name */
size_t name_len;
const char *name = lua_tolstring(L, 3, &name_len);
- luamp_encode_uint(cfg, &stream, IPROTO_FUNCTION_NAME);
- luamp_encode_str(cfg, &stream, name, name_len);
+ mpstream_encode_uint(&stream, IPROTO_FUNCTION_NAME);
+ mpstream_encode_strn(&stream, name, name_len);
/* encode args */
- luamp_encode_uint(cfg, &stream, IPROTO_TUPLE);
+ mpstream_encode_uint(&stream, IPROTO_TUPLE);
luamp_encode_tuple(L, cfg, &stream, 4);
netbox_encode_request(&stream, svp);
@@ -205,16 +206,16 @@ netbox_encode_eval(lua_State *L)
struct mpstream stream;
size_t svp = netbox_prepare_request(L, &stream, IPROTO_EVAL);
- luamp_encode_map(cfg, &stream, 2);
+ mpstream_encode_map(&stream, 2);
/* encode expr */
size_t expr_len;
const char *expr = lua_tolstring(L, 3, &expr_len);
- luamp_encode_uint(cfg, &stream, IPROTO_EXPR);
- luamp_encode_str(cfg, &stream, expr, expr_len);
+ mpstream_encode_uint(&stream, IPROTO_EXPR);
+ mpstream_encode_strn(&stream, expr, expr_len);
/* encode args */
- luamp_encode_uint(cfg, &stream, IPROTO_TUPLE);
+ mpstream_encode_uint(&stream, IPROTO_TUPLE);
luamp_encode_tuple(L, cfg, &stream, 4);
netbox_encode_request(&stream, svp);
@@ -233,7 +234,7 @@ netbox_encode_select(lua_State *L)
struct mpstream stream;
size_t svp = netbox_prepare_request(L, &stream, IPROTO_SELECT);
- luamp_encode_map(cfg, &stream, 6);
+ mpstream_encode_map(&stream, 6);
uint32_t space_id = lua_tonumber(L, 3);
uint32_t index_id = lua_tonumber(L, 4);
@@ -242,27 +243,27 @@ netbox_encode_select(lua_State *L)
uint32_t limit = lua_tonumber(L, 7);
/* encode space_id */
- luamp_encode_uint(cfg, &stream, IPROTO_SPACE_ID);
- luamp_encode_uint(cfg, &stream, space_id);
+ mpstream_encode_uint(&stream, IPROTO_SPACE_ID);
+ mpstream_encode_uint(&stream, space_id);
/* encode index_id */
- luamp_encode_uint(cfg, &stream, IPROTO_INDEX_ID);
- luamp_encode_uint(cfg, &stream, index_id);
+ mpstream_encode_uint(&stream, IPROTO_INDEX_ID);
+ mpstream_encode_uint(&stream, index_id);
/* encode iterator */
- luamp_encode_uint(cfg, &stream, IPROTO_ITERATOR);
- luamp_encode_uint(cfg, &stream, iterator);
+ mpstream_encode_uint(&stream, IPROTO_ITERATOR);
+ mpstream_encode_uint(&stream, iterator);
/* encode offset */
- luamp_encode_uint(cfg, &stream, IPROTO_OFFSET);
- luamp_encode_uint(cfg, &stream, offset);
+ mpstream_encode_uint(&stream, IPROTO_OFFSET);
+ mpstream_encode_uint(&stream, offset);
/* encode limit */
- luamp_encode_uint(cfg, &stream, IPROTO_LIMIT);
- luamp_encode_uint(cfg, &stream, limit);
+ mpstream_encode_uint(&stream, IPROTO_LIMIT);
+ mpstream_encode_uint(&stream, limit);
/* encode key */
- luamp_encode_uint(cfg, &stream, IPROTO_KEY);
+ mpstream_encode_uint(&stream, IPROTO_KEY);
luamp_convert_key(L, cfg, &stream, 8);
netbox_encode_request(&stream, svp);
@@ -279,15 +280,15 @@ netbox_encode_insert_or_replace(lua_State *L, uint32_t reqtype)
struct mpstream stream;
size_t svp = netbox_prepare_request(L, &stream, reqtype);
- luamp_encode_map(cfg, &stream, 2);
+ mpstream_encode_map(&stream, 2);
/* encode space_id */
uint32_t space_id = lua_tonumber(L, 3);
- luamp_encode_uint(cfg, &stream, IPROTO_SPACE_ID);
- luamp_encode_uint(cfg, &stream, space_id);
+ mpstream_encode_uint(&stream, IPROTO_SPACE_ID);
+ mpstream_encode_uint(&stream, space_id);
/* encode args */
- luamp_encode_uint(cfg, &stream, IPROTO_TUPLE);
+ mpstream_encode_uint(&stream, IPROTO_TUPLE);
luamp_encode_tuple(L, cfg, &stream, 4);
netbox_encode_request(&stream, svp);
@@ -317,20 +318,20 @@ netbox_encode_delete(lua_State *L)
struct mpstream stream;
size_t svp = netbox_prepare_request(L, &stream, IPROTO_DELETE);
- luamp_encode_map(cfg, &stream, 3);
+ mpstream_encode_map(&stream, 3);
/* encode space_id */
uint32_t space_id = lua_tonumber(L, 3);
- luamp_encode_uint(cfg, &stream, IPROTO_SPACE_ID);
- luamp_encode_uint(cfg, &stream, space_id);
+ mpstream_encode_uint(&stream, IPROTO_SPACE_ID);
+ mpstream_encode_uint(&stream, space_id);
/* encode space_id */
uint32_t index_id = lua_tonumber(L, 4);
- luamp_encode_uint(cfg, &stream, IPROTO_INDEX_ID);
- luamp_encode_uint(cfg, &stream, index_id);
+ mpstream_encode_uint(&stream, IPROTO_INDEX_ID);
+ mpstream_encode_uint(&stream, index_id);
/* encode key */
- luamp_encode_uint(cfg, &stream, IPROTO_KEY);
+ mpstream_encode_uint(&stream, IPROTO_KEY);
luamp_convert_key(L, cfg, &stream, 5);
netbox_encode_request(&stream, svp);
@@ -348,30 +349,30 @@ netbox_encode_update(lua_State *L)
struct mpstream stream;
size_t svp = netbox_prepare_request(L, &stream, IPROTO_UPDATE);
- luamp_encode_map(cfg, &stream, 5);
+ mpstream_encode_map(&stream, 5);
/* encode space_id */
uint32_t space_id = lua_tonumber(L, 3);
- luamp_encode_uint(cfg, &stream, IPROTO_SPACE_ID);
- luamp_encode_uint(cfg, &stream, space_id);
+ mpstream_encode_uint(&stream, IPROTO_SPACE_ID);
+ mpstream_encode_uint(&stream, space_id);
/* encode index_id */
uint32_t index_id = lua_tonumber(L, 4);
- luamp_encode_uint(cfg, &stream, IPROTO_INDEX_ID);
- luamp_encode_uint(cfg, &stream, index_id);
+ mpstream_encode_uint(&stream, IPROTO_INDEX_ID);
+ mpstream_encode_uint(&stream, index_id);
/* encode index_id */
- luamp_encode_uint(cfg, &stream, IPROTO_INDEX_BASE);
- luamp_encode_uint(cfg, &stream, 1);
+ mpstream_encode_uint(&stream, IPROTO_INDEX_BASE);
+ mpstream_encode_uint(&stream, 1);
/* encode in reverse order for speedup - see luamp_encode() code */
/* encode ops */
- luamp_encode_uint(cfg, &stream, IPROTO_TUPLE);
+ mpstream_encode_uint(&stream, IPROTO_TUPLE);
luamp_encode_tuple(L, cfg, &stream, 6);
lua_pop(L, 1); /* ops */
/* encode key */
- luamp_encode_uint(cfg, &stream, IPROTO_KEY);
+ mpstream_encode_uint(&stream, IPROTO_KEY);
luamp_convert_key(L, cfg, &stream, 5);
netbox_encode_request(&stream, svp);
@@ -389,25 +390,25 @@ netbox_encode_upsert(lua_State *L)
struct mpstream stream;
size_t svp = netbox_prepare_request(L, &stream, IPROTO_UPSERT);
- luamp_encode_map(cfg, &stream, 4);
+ mpstream_encode_map(&stream, 4);
/* encode space_id */
uint32_t space_id = lua_tonumber(L, 3);
- luamp_encode_uint(cfg, &stream, IPROTO_SPACE_ID);
- luamp_encode_uint(cfg, &stream, space_id);
+ mpstream_encode_uint(&stream, IPROTO_SPACE_ID);
+ mpstream_encode_uint(&stream, space_id);
/* encode index_base */
- luamp_encode_uint(cfg, &stream, IPROTO_INDEX_BASE);
- luamp_encode_uint(cfg, &stream, 1);
+ mpstream_encode_uint(&stream, IPROTO_INDEX_BASE);
+ mpstream_encode_uint(&stream, 1);
/* encode in reverse order for speedup - see luamp_encode() code */
/* encode ops */
- luamp_encode_uint(cfg, &stream, IPROTO_OPS);
+ mpstream_encode_uint(&stream, IPROTO_OPS);
luamp_encode_tuple(L, cfg, &stream, 5);
lua_pop(L, 1); /* ops */
/* encode tuple */
- luamp_encode_uint(cfg, &stream, IPROTO_TUPLE);
+ mpstream_encode_uint(&stream, IPROTO_TUPLE);
luamp_encode_tuple(L, cfg, &stream, 4);
netbox_encode_request(&stream, svp);
@@ -566,17 +567,17 @@ netbox_encode_execute(lua_State *L)
struct mpstream stream;
size_t svp = netbox_prepare_request(L, &stream, IPROTO_EXECUTE);
- luamp_encode_map(cfg, &stream, 3);
+ mpstream_encode_map(&stream, 3);
size_t len;
const char *query = lua_tolstring(L, 3, &len);
- luamp_encode_uint(cfg, &stream, IPROTO_SQL_TEXT);
- luamp_encode_str(cfg, &stream, query, len);
+ mpstream_encode_uint(&stream, IPROTO_SQL_TEXT);
+ mpstream_encode_strn(&stream, query, len);
- luamp_encode_uint(cfg, &stream, IPROTO_SQL_BIND);
+ mpstream_encode_uint(&stream, IPROTO_SQL_BIND);
luamp_encode_tuple(L, cfg, &stream, 4);
- luamp_encode_uint(cfg, &stream, IPROTO_OPTIONS);
+ mpstream_encode_uint(&stream, IPROTO_OPTIONS);
luamp_encode_tuple(L, cfg, &stream, 5);
netbox_encode_request(&stream, svp);
diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index 22fe696..126466c 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -42,6 +42,7 @@
#include "box/tuple_convert.h"
#include "box/errcode.h"
#include "json/path.h"
+#include "mpstream.h"
/** {{{ box.tuple Lua library
*
@@ -111,7 +112,7 @@ lbox_tuple_new(lua_State *L)
luamp_encode_tuple(L, luaL_msgpack_default, &stream, 1);
} else {
/* Backward-compatible format: box.tuple.new(1, 2, 3). */
- luamp_encode_array(luaL_msgpack_default, &stream, argc);
+ mpstream_encode_array(&stream, argc);
for (int k = 1; k <= argc; ++k) {
luamp_encode(L, luaL_msgpack_default, &stream, k);
}
@@ -228,9 +229,9 @@ luamp_convert_key(struct lua_State *L, struct luaL_serializer *cfg,
luamp_encode_r(L, cfg, stream, &field, 0);
lua_pop(L, 1);
} else if (field.type == MP_NIL) {
- luamp_encode_array(cfg, stream, 0);
+ mpstream_encode_array(stream, 0);
} else {
- luamp_encode_array(cfg, stream, 1);
+ mpstream_encode_array(stream, 1);
lua_pushvalue(L, index);
luamp_encode_r(L, cfg, stream, &field, 0);
lua_pop(L, 1);
@@ -393,18 +394,18 @@ lbox_tuple_transform(struct lua_State *L)
/*
* Prepare UPDATE expression
*/
- luamp_encode_array(luaL_msgpack_default, &stream, op_cnt);
+ mpstream_encode_array(&stream, op_cnt);
if (len > 0) {
- luamp_encode_array(luaL_msgpack_default, &stream, 3);
- luamp_encode_str(luaL_msgpack_default, &stream, "#", 1);
- luamp_encode_uint(luaL_msgpack_default, &stream, offset);
- luamp_encode_uint(luaL_msgpack_default, &stream, len);
+ mpstream_encode_array(&stream, 3);
+ mpstream_encode_str(&stream, "#");
+ mpstream_encode_uint(&stream, offset);
+ mpstream_encode_uint(&stream, len);
}
for (int i = argc ; i > 3; i--) {
- luamp_encode_array(luaL_msgpack_default, &stream, 3);
- luamp_encode_str(luaL_msgpack_default, &stream, "!", 1);
- luamp_encode_uint(luaL_msgpack_default, &stream, offset);
+ mpstream_encode_array(&stream, 3);
+ mpstream_encode_str(&stream, "!");
+ mpstream_encode_uint(&stream, offset);
luamp_encode(L, luaL_msgpack_default, &stream, i);
}
mpstream_flush(&stream);
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index acd860a..b470060 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -29,7 +29,7 @@
* SUCH DAMAGE.
*/
#include "lua/msgpack.h"
-
+#include "mpstream.h"
#include "lua/utils.h"
#if defined(LUAJIT)
@@ -50,46 +50,6 @@ luamp_error(void *error_ctx)
luaL_error(L, diag_last_error(diag_get())->errmsg);
}
-void
-mpstream_init(struct mpstream *stream, void *ctx,
- luamp_reserve_f reserve, luamp_alloc_f alloc,
- luamp_error_f error, void *error_ctx)
-{
- stream->ctx = ctx;
- stream->reserve = reserve;
- stream->alloc = alloc;
- stream->error = error;
- stream->error_ctx = error_ctx;
- mpstream_reset(stream);
-}
-
-void
-mpstream_reserve_slow(struct mpstream *stream, size_t size)
-{
- stream->alloc(stream->ctx, stream->pos - stream->buf);
- stream->buf = (char *) stream->reserve(stream->ctx, &size);
- if (stream->buf == NULL) {
- diag_set(OutOfMemory, size, "mpstream", "reserve");
- stream->error(stream->error_ctx);
- }
- stream->pos = stream->buf;
- stream->end = stream->pos + size;
-}
-
-void
-mpstream_reset(struct mpstream *stream)
-{
- size_t size = 0;
- stream->buf = (char *) stream->reserve(stream->ctx, &size);
- if (stream->buf == NULL) {
- diag_set(OutOfMemory, size, "mpstream", "reset");
- stream->error(stream->error_ctx);
- }
- stream->pos = stream->buf;
- stream->end = stream->pos + size;
-}
-
-
static uint32_t CTID_CHAR_PTR;
static uint32_t CTID_STRUCT_IBUF;
@@ -107,104 +67,6 @@ static luamp_encode_extension_f luamp_encode_extension =
static luamp_decode_extension_f luamp_decode_extension =
luamp_decode_extension_default;
-void
-luamp_encode_array(struct luaL_serializer *cfg, struct mpstream *stream,
- uint32_t size)
-{
- (void) cfg;
- assert(mp_sizeof_array(size) <= 5);
- char *data = mpstream_reserve(stream, 5);
- char *pos = mp_encode_array(data, size);
- mpstream_advance(stream, pos - data);
-}
-
-void
-luamp_encode_map(struct luaL_serializer *cfg, struct mpstream *stream,
- uint32_t size)
-{
- (void) cfg;
- assert(mp_sizeof_map(size) <= 5);
- char *data = mpstream_reserve(stream, 5);
- char *pos = mp_encode_map(data, size);
- mpstream_advance(stream, pos - data);
-}
-
-void
-luamp_encode_uint(struct luaL_serializer *cfg, struct mpstream *stream,
- uint64_t num)
-{
- (void) cfg;
- assert(mp_sizeof_uint(num) <= 9);
- char *data = mpstream_reserve(stream, 9);
- char *pos = mp_encode_uint(data, num);
- mpstream_advance(stream, pos - data);
-}
-
-void
-luamp_encode_int(struct luaL_serializer *cfg, struct mpstream *stream,
- int64_t num)
-{
- (void) cfg;
- assert(mp_sizeof_int(num) <= 9);
- char *data = mpstream_reserve(stream, 9);
- char *pos = mp_encode_int(data, num);
- mpstream_advance(stream, pos - data);
-}
-
-void
-luamp_encode_float(struct luaL_serializer *cfg, struct mpstream *stream,
- float num)
-{
- (void) cfg;
- assert(mp_sizeof_float(num) <= 5);
- char *data = mpstream_reserve(stream, 5);
- char *pos = mp_encode_float(data, num);
- mpstream_advance(stream, pos - data);
-}
-
-void
-luamp_encode_double(struct luaL_serializer *cfg, struct mpstream *stream,
- double num)
-{
- (void) cfg;
- assert(mp_sizeof_double(num) <= 9);
- char *data = mpstream_reserve(stream, 9);
- char *pos = mp_encode_double(data, num);
- mpstream_advance(stream, pos - data);
-}
-
-void
-luamp_encode_str(struct luaL_serializer *cfg, struct mpstream *stream,
- const char *str, uint32_t len)
-{
- (void) cfg;
- assert(mp_sizeof_str(len) <= 5 + len);
- char *data = mpstream_reserve(stream, 5 + len);
- char *pos = mp_encode_str(data, str, len);
- mpstream_advance(stream, pos - data);
-}
-
-void
-luamp_encode_nil(struct luaL_serializer *cfg, struct mpstream *stream)
-{
- (void) cfg;
- assert(mp_sizeof_nil() <= 1);
- char *data = mpstream_reserve(stream, 1);
- char *pos = mp_encode_nil(data);
- mpstream_advance(stream, pos - data);
-}
-
-void
-luamp_encode_bool(struct luaL_serializer *cfg, struct mpstream *stream,
- bool val)
-{
- (void) cfg;
- assert(mp_sizeof_bool(val) <= 1);
- char *data = mpstream_reserve(stream, 1);
- char *pos = mp_encode_bool(data, val);
- mpstream_advance(stream, pos - data);
-}
-
static enum mp_type
luamp_encode_extension_default(struct lua_State *L, int idx,
struct mpstream *stream)
@@ -254,38 +116,36 @@ luamp_encode_r(struct lua_State *L, struct luaL_serializer *cfg,
restart: /* used by MP_EXT */
switch (field->type) {
case MP_UINT:
- luamp_encode_uint(cfg, stream, field->ival);
+ mpstream_encode_uint(stream, field->ival);
return MP_UINT;
case MP_STR:
- luamp_encode_str(cfg, stream, field->sval.data,
- field->sval.len);
+ mpstream_encode_strn(stream, field->sval.data, field->sval.len);
return MP_STR;
case MP_BIN:
- luamp_encode_str(cfg, stream, field->sval.data,
- field->sval.len);
+ mpstream_encode_strn(stream, field->sval.data, field->sval.len);
return MP_BIN;
case MP_INT:
- luamp_encode_int(cfg, stream, field->ival);
+ mpstream_encode_int(stream, field->ival);
return MP_INT;
case MP_FLOAT:
- luamp_encode_float(cfg, stream, field->fval);
+ mpstream_encode_float(stream, field->fval);
return MP_FLOAT;
case MP_DOUBLE:
- luamp_encode_double(cfg, stream, field->dval);
+ mpstream_encode_double(stream, field->dval);
return MP_DOUBLE;
case MP_BOOL:
- luamp_encode_bool(cfg, stream, field->bval);
+ mpstream_encode_bool(stream, field->bval);
return MP_BOOL;
case MP_NIL:
- luamp_encode_nil(cfg, stream);
+ mpstream_encode_nil(stream);
return MP_NIL;
case MP_MAP:
/* Map */
if (level >= cfg->encode_max_depth) {
- luamp_encode_nil(cfg, stream); /* Limit nested maps */
+ mpstream_encode_nil(stream); /* Limit nested maps */
return MP_NIL;
}
- luamp_encode_map(cfg, stream, field->size);
+ mpstream_encode_map(stream, field->size);
lua_pushnil(L); /* first key */
while (lua_next(L, top) != 0) {
lua_pushvalue(L, -2); /* push a copy of key to top */
@@ -301,11 +161,11 @@ restart: /* used by MP_EXT */
case MP_ARRAY:
/* Array */
if (level >= cfg->encode_max_depth) {
- luamp_encode_nil(cfg, stream); /* Limit nested arrays */
+ mpstream_encode_nil(stream); /* Limit nested arrays */
return MP_NIL;
}
uint32_t size = field->size;
- luamp_encode_array(cfg, stream, size);
+ mpstream_encode_array(stream, size);
for (uint32_t i = 0; i < size; i++) {
lua_rawgeti(L, top, i + 1);
luaL_tofield(L, cfg, top + 1, field);
diff --git a/src/lua/msgpack.h b/src/lua/msgpack.h
index bacf3e0..d0ade30 100644
--- a/src/lua/msgpack.h
+++ b/src/lua/msgpack.h
@@ -1,7 +1,7 @@
#ifndef TARANTOOL_LUA_MSGPACK_H_INCLUDED
#define TARANTOOL_LUA_MSGPACK_H_INCLUDED
/*
- * Copyright 2010-2015, Tarantool AUTHORS, please see AUTHORS file.
+ * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
*
* Redistribution and use in source and binary forms, with or
* without modification, are permitted provided that the following
@@ -42,6 +42,8 @@ extern "C" {
#include <lua.h>
struct luaL_serializer;
+struct mpstream;
+
/**
* Default instance of msgpack serializer (msgpack = require('msgpack')).
* This instance is used by all box's Lua/C API bindings (e.g. space:replace()).
@@ -51,111 +53,13 @@ struct luaL_serializer;
extern struct luaL_serializer *luaL_msgpack_default;
/**
- * A streaming API so that it's possible to encode to any output
- * stream.
- */
-
-/**
- * Ask the allocator to reserve at least size bytes. It can reserve
- * more, and update *size with the new size.
- */
-typedef void *(*luamp_reserve_f)(void *ctx, size_t *size);
-
-/** Actually use the bytes. */
-typedef void *(*luamp_alloc_f)(void *ctx, size_t size);
-
-/** Actually use the bytes. */
-typedef void (*luamp_error_f)(void *error_ctx);
-
-struct mpstream {
- /**
- * When pos >= end, or required size doesn't fit in
- * pos..end range alloc() is called to advance the stream
- * and reserve() to get a new chunk.
- */
- char *buf, *pos, *end;
- void *ctx;
- luamp_reserve_f reserve;
- luamp_alloc_f alloc;
- luamp_error_f error;
- void *error_ctx;
-};
-
-/**
* luaL_error()
*/
void
luamp_error(void *);
-void
-mpstream_init(struct mpstream *stream, void *ctx,
- luamp_reserve_f reserve, luamp_alloc_f alloc,
- luamp_error_f error, void *error_ctx);
-
-void
-mpstream_reset(struct mpstream *stream);
-
-void
-mpstream_reserve_slow(struct mpstream *stream, size_t size);
-
-static inline void
-mpstream_flush(struct mpstream *stream)
-{
- stream->alloc(stream->ctx, stream->pos - stream->buf);
- stream->buf = stream->pos;
-}
-
-static inline char *
-mpstream_reserve(struct mpstream *stream, size_t size)
-{
- if (stream->pos + size > stream->end)
- mpstream_reserve_slow(stream, size);
- return stream->pos;
-}
-
-static inline void
-mpstream_advance(struct mpstream *stream, size_t size)
-{
- assert(stream->pos + size <= stream->end);
- stream->pos += size;
-}
-
enum { LUAMP_ALLOC_FACTOR = 256 };
-void
-luamp_encode_array(struct luaL_serializer *cfg, struct mpstream *stream,
- uint32_t size);
-
-void
-luamp_encode_map(struct luaL_serializer *cfg, struct mpstream *stream, uint32_t size);
-
-void
-luamp_encode_uint(struct luaL_serializer *cfg, struct mpstream *stream,
- uint64_t num);
-
-void
-luamp_encode_int(struct luaL_serializer *cfg, struct mpstream *stream,
- int64_t num);
-
-void
-luamp_encode_float(struct luaL_serializer *cfg, struct mpstream *stream,
- float num);
-
-void
-luamp_encode_double(struct luaL_serializer *cfg, struct mpstream *stream,
- double num);
-
-void
-luamp_encode_str(struct luaL_serializer *cfg, struct mpstream *stream,
- const char *str, uint32_t len);
-
-void
-luamp_encode_nil(struct luaL_serializer *cfg, struct mpstream *stream);
-
-void
-luamp_encode_bool(struct luaL_serializer *cfg, struct mpstream *stream,
- bool val);
-
/* low-level function needed for execute_lua_call() */
enum mp_type
luamp_encode_r(struct lua_State *L, struct luaL_serializer *cfg,
diff --git a/src/mpstream.c b/src/mpstream.c
new file mode 100644
index 0000000..23d27f9
--- /dev/null
+++ b/src/mpstream.c
@@ -0,0 +1,205 @@
+/*
+ * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "mpstream.h"
+#include <assert.h>
+#include <stdint.h>
+#include "msgpuck.h"
+
+void
+mpstream_reserve_slow(struct mpstream *stream, size_t size)
+{
+ stream->alloc(stream->ctx, stream->pos - stream->buf);
+ stream->buf = (char *) stream->reserve(stream->ctx, &size);
+ if (stream->buf == NULL) {
+ diag_set(OutOfMemory, size, "mpstream", "reserve");
+ stream->error(stream->error_ctx);
+ }
+ stream->pos = stream->buf;
+ stream->end = stream->pos + size;
+}
+
+void
+mpstream_reset(struct mpstream *stream)
+{
+ size_t size = 0;
+ stream->buf = (char *) stream->reserve(stream->ctx, &size);
+ if (stream->buf == NULL) {
+ diag_set(OutOfMemory, size, "mpstream", "reset");
+ stream->error(stream->error_ctx);
+ }
+ stream->pos = stream->buf;
+ stream->end = stream->pos + size;
+}
+
+/**
+ * A streaming API so that it's possible to encode to any output
+ * stream.
+ */
+void
+mpstream_init(struct mpstream *stream, void *ctx,
+ mpstream_reserve_f reserve, mpstream_alloc_f alloc,
+ mpstream_error_f error, void *error_ctx)
+{
+ stream->ctx = ctx;
+ stream->reserve = reserve;
+ stream->alloc = alloc;
+ stream->error = error;
+ stream->error_ctx = error_ctx;
+ mpstream_reset(stream);
+}
+
+void
+mpstream_flush(struct mpstream *stream)
+{
+ stream->alloc(stream->ctx, stream->pos - stream->buf);
+ stream->buf = stream->pos;
+}
+
+char *
+mpstream_reserve(struct mpstream *stream, size_t size)
+{
+ if (stream->pos + size > stream->end)
+ mpstream_reserve_slow(stream, size);
+ return stream->pos;
+}
+
+void
+mpstream_advance(struct mpstream *stream, size_t size)
+{
+ assert(stream->pos + size <= stream->end);
+ stream->pos += size;
+}
+
+void
+mpstream_encode_array(struct mpstream *stream, uint32_t size)
+{
+ assert(mp_sizeof_array(size) <= 5);
+ char *data = mpstream_reserve(stream, 5);
+ if (data == NULL)
+ return;
+ char *pos = mp_encode_array(data, size);
+ mpstream_advance(stream, pos - data);
+}
+
+void
+mpstream_encode_map(struct mpstream *stream, uint32_t size)
+{
+ assert(mp_sizeof_map(size) <= 5);
+ char *data = mpstream_reserve(stream, 5);
+ if (data == NULL)
+ return;
+ char *pos = mp_encode_map(data, size);
+ mpstream_advance(stream, pos - data);
+}
+
+void
+mpstream_encode_uint(struct mpstream *stream, uint64_t num)
+{
+ assert(mp_sizeof_uint(num) <= 9);
+ char *data = mpstream_reserve(stream, 9);
+ if (data == NULL)
+ return;
+ char *pos = mp_encode_uint(data, num);
+ mpstream_advance(stream, pos - data);
+}
+
+void
+mpstream_encode_int(struct mpstream *stream, int64_t num)
+{
+ assert(mp_sizeof_int(num) <= 9);
+ char *data = mpstream_reserve(stream, 9);
+ if (data == NULL)
+ return;
+ char *pos = mp_encode_int(data, num);
+ mpstream_advance(stream, pos - data);
+}
+
+void
+mpstream_encode_float(struct mpstream *stream, float num)
+{
+ assert(mp_sizeof_float(num) <= 5);
+ char *data = mpstream_reserve(stream, 5);
+ if (data == NULL)
+ return;
+ char *pos = mp_encode_float(data, num);
+ mpstream_advance(stream, pos - data);
+}
+
+void
+mpstream_encode_double(struct mpstream *stream, double num)
+{
+ assert(mp_sizeof_double(num) <= 9);
+ char *data = mpstream_reserve(stream, 9);
+ char *pos = mp_encode_double(data, num);
+ if (data == NULL)
+ return;
+ mpstream_advance(stream, pos - data);
+}
+
+void
+mpstream_encode_strn(struct mpstream *stream, const char *str, uint32_t len)
+{
+ assert(mp_sizeof_str(len) <= 5 + len);
+ char *data = mpstream_reserve(stream, 5 + len);
+ if (data == NULL)
+ return;
+ char *pos = mp_encode_str(data, str, len);
+ mpstream_advance(stream, pos - data);
+}
+
+void
+mpstream_encode_str(struct mpstream *stream, const char *str)
+{
+ mpstream_encode_strn(stream, str, strlen(str));
+}
+
+void
+mpstream_encode_nil(struct mpstream *stream)
+{
+ assert(mp_sizeof_nil() <= 1);
+ char *data = mpstream_reserve(stream, 1);
+ if (data == NULL)
+ return;
+ char *pos = mp_encode_nil(data);
+ mpstream_advance(stream, pos - data);
+}
+
+void
+mpstream_encode_bool(struct mpstream *stream, bool val)
+{
+ assert(mp_sizeof_bool(val) <= 1);
+ char *data = mpstream_reserve(stream, 1);
+ if (data == NULL)
+ return;
+ char *pos = mp_encode_bool(data, val);
+ mpstream_advance(stream, pos - data);
+}
diff --git a/src/mpstream.h b/src/mpstream.h
new file mode 100644
index 0000000..789bd74
--- /dev/null
+++ b/src/mpstream.h
@@ -0,0 +1,124 @@
+#ifndef TARANTOOL_LUA_MPSTREAM_H_INCLUDED
+#define TARANTOOL_LUA_MPSTREAM_H_INCLUDED
+/*
+ * Copyright 2010-2015, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "diag.h"
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+/**
+* Ask the allocator to reserve at least size bytes. It can reserve
+* more, and update *size with the new size.
+*/
+typedef void *(*mpstream_reserve_f)(void *ctx, size_t *size);
+
+/** Actually use the bytes. */
+typedef void *(*mpstream_alloc_f)(void *ctx, size_t size);
+
+/** Actually use the bytes. */
+typedef void (*mpstream_error_f)(void *error_ctx);
+
+struct mpstream {
+ /**
+ * When pos >= end, or required size doesn't fit in
+ * pos..end range alloc() is called to advance the stream
+ * and reserve() to get a new chunk.
+ */
+ char *buf, *pos, *end;
+ void *ctx;
+ mpstream_reserve_f reserve;
+ mpstream_alloc_f alloc;
+ mpstream_error_f error;
+ void *error_ctx;
+};
+
+void
+mpstream_reserve_slow(struct mpstream *stream, size_t size);
+
+void
+mpstream_reset(struct mpstream *stream);
+
+/**
+ * A streaming API so that it's possible to encode to any output
+ * stream.
+ */
+void
+mpstream_init(struct mpstream *stream, void *ctx,
+ mpstream_reserve_f reserve, mpstream_alloc_f alloc,
+ mpstream_error_f error, void *error_ctx);
+
+void
+mpstream_flush(struct mpstream *stream);
+
+char *
+mpstream_reserve(struct mpstream *stream, size_t size);
+
+void
+mpstream_advance(struct mpstream *stream, size_t size);
+
+void
+mpstream_encode_array(struct mpstream *stream, uint32_t size);
+
+void
+mpstream_encode_map(struct mpstream *stream, uint32_t size);
+
+void
+mpstream_encode_uint(struct mpstream *stream, uint64_t num);
+
+void
+mpstream_encode_int(struct mpstream *stream, int64_t num);
+
+void
+mpstream_encode_float(struct mpstream *stream, float num);
+
+void
+mpstream_encode_double(struct mpstream *stream, double num);
+
+void
+mpstream_encode_strn(struct mpstream *stream, const char *str, uint32_t len);
+
+void
+mpstream_encode_str(struct mpstream *stream, const char *str);
+
+void
+mpstream_encode_nil(struct mpstream *stream);
+
+void
+mpstream_encode_bool(struct mpstream *stream, bool val);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
+
+#endif /* TARANTOOL_LUA_MPSTREAM_H_INCLUDED */
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] [PATCH v2 2/2] sql: remove struct Enc
2018-08-27 11:11 [tarantool-patches] [PATCH v2 0/2] sql: remove struct Enc Kirill Shcherbatov
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 1/2] box: export mpstream methods to core Kirill Shcherbatov
@ 2018-08-27 11:11 ` Kirill Shcherbatov
2018-08-28 1:43 ` [tarantool-patches] " Vladislav Shpilevoy
2018-08-28 1:43 ` [tarantool-patches] Re: [PATCH v2 0/2] " Vladislav Shpilevoy
2018-08-29 14:12 ` Kirill Yukhin
3 siblings, 1 reply; 10+ messages in thread
From: Kirill Shcherbatov @ 2018-08-27 11:11 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov
We shouldn't use legacy Enc structure in SQL as we have
featured class mpstream in tarantool core. With this path
all allocations temporally commited with mpstream initialized
on region and then duplicated in dynamic persistent memory via
sqlite3DbMallocRaw.
Closes #3545.
---
src/box/sql.c | 473 +++++++++++++++++++++++----------------------
src/box/sql/build.c | 156 ++++++++-------
src/box/sql/tarantoolInt.h | 80 +++++---
src/box/sql/trigger.c | 62 +++---
4 files changed, 416 insertions(+), 355 deletions(-)
diff --git a/src/box/sql.c b/src/box/sql.c
index b98593b..6bf832b 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -56,6 +56,8 @@
#include "xrow.h"
#include "iproto_constants.h"
#include "fkey.h"
+#include "mpstream.h"
+#include "small/region.h"
static sqlite3 *db = NULL;
@@ -840,6 +842,32 @@ rename_fail:
return SQL_TARANTOOL_ERROR;
}
+/** Callback to forward and error from mpstream methods. */
+static void
+set_encode_error(void *error_ctx)
+{
+ *(bool *)error_ctx = true;
+}
+
+void
+mpstream_encode_index_opts(struct mpstream *stream, struct index_opts *opts)
+{
+ mpstream_encode_map(stream, 2);
+ /* Mark as unique pk and unique indexes */
+ mpstream_encode_str(stream, "unique");
+ /* If user didn't defined ON CONFLICT OPTIONS, all uniqueness checks
+ * will be made by Tarantool. However, Tarantool doesn't have ON
+ * CONFLIT option, so in that case (except ON CONFLICT ABORT, which is
+ * default behavior) uniqueness will be checked by SQL.
+ * INSERT OR REPLACE/IGNORE uniqueness checks will be also done by
+ * Tarantool.
+ */
+ mpstream_encode_bool(stream, opts->is_unique);
+ mpstream_encode_str(stream, "sql");
+ mpstream_encode_strn(stream, opts->sql,
+ opts->sql != NULL ? strlen(opts->sql) : 0);
+}
+
int
sql_index_update_table_name(struct index_def *def, const char *new_tbl_name,
char **sql_stmt)
@@ -851,33 +879,43 @@ sql_index_update_table_name(struct index_def *def, const char *new_tbl_name,
if (*sql_stmt == NULL)
return -1;
- uint32_t key_len = mp_sizeof_uint(def->space_id) +
- mp_sizeof_uint(def->iid) + mp_sizeof_array(2);
+ struct region *region = &fiber()->gc;
+ size_t used = region_used(region);
+ struct mpstream stream;
+ bool is_error = false;
+ mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
+ set_encode_error, &is_error);
+
+ /* Encode key. */
+ mpstream_encode_array(&stream, 2);
+ mpstream_encode_uint(&stream, def->space_id);
+ mpstream_encode_uint(&stream, def->iid);
+
+ /* Encode op. */
+ uint32_t op_offset = stream.pos - stream.buf;
+ mpstream_encode_array(&stream, 1);
+ mpstream_encode_array(&stream, 3);
+ mpstream_encode_str(&stream, "=");
+ mpstream_encode_uint(&stream, BOX_INDEX_FIELD_OPTS);
+
+ /* Encode index opts. */
struct index_opts opts = def->opts;
opts.sql = *sql_stmt;
- uint32_t new_opts_sz = sql_encode_index_opts(&opts, NULL);
- uint32_t op_sz = mp_sizeof_array(1) + mp_sizeof_array(3) +
- mp_sizeof_str(1) + mp_sizeof_uint(4) + new_opts_sz;
+ mpstream_encode_index_opts(&stream, &opts);
- char *key_begin = (char*) region_alloc(&fiber()->gc, key_len + op_sz);
- if (key_begin == NULL) {
- diag_set(OutOfMemory, key_len + op_sz, "region_alloc",
- "key_begin");
+ mpstream_flush(&stream);
+ if (is_error) {
+ diag_set(OutOfMemory, stream.pos - stream.buf,
+ "mpstream_flush", "stream");
return -1;
}
- char *key = mp_encode_array(key_begin, 2);
- key = mp_encode_uint(key, def->space_id);
- key = mp_encode_uint(key, def->iid);
-
- char *op_begin = key;
- char *op = mp_encode_array(op_begin, 1);
- op = mp_encode_array(op, 3);
- op = mp_encode_str(op, "=", 1);
- op = mp_encode_uint(op, BOX_INDEX_FIELD_OPTS);
- op += sql_encode_index_opts(&opts, op);
-
- if (box_update(BOX_INDEX_ID, 0, key_begin, key, op_begin, op,
- 0, NULL) != 0)
+ size_t sz = region_used(region) - used;
+ char *raw = region_join(region, sz);
+ if (raw == NULL)
+ diag_set(OutOfMemory, sz, "region_join", "raw");
+
+ if (box_update(BOX_INDEX_ID, 0, raw, raw + op_offset, raw + op_offset,
+ raw + sz, 0, NULL) != 0)
return -1;
return 0;
@@ -1233,66 +1271,6 @@ void tarantoolSqlite3LoadSchema(struct init_data *init)
*/
/*
- * Resulting data is of the variable length. Routines are called twice:
- * 1. with a NULL buffer, yielding result size estimation;
- * 2. with a buffer of the estimated size, rendering the result.
- *
- * For convenience, formatting routines use Enc structure to call
- * Enc is either configured to perform size estimation
- * or to render the result.
- */
-struct Enc {
- char *(*encode_uint)(char *data, uint64_t num);
- char *(*encode_str)(char *data, const char *str, uint32_t len);
- char *(*encode_bool)(char *data, bool v);
- char *(*encode_array)(char *data, uint32_t len);
- char *(*encode_map)(char *data, uint32_t len);
-};
-
-/* no_encode_XXX functions estimate result size */
-
-static char *no_encode_uint(char *data, uint64_t num)
-{
- /* MsgPack UINT is encoded in 9 bytes or less */
- (void)num; return data + 9;
-}
-
-static char *no_encode_str(char *data, const char *str, uint32_t len)
-{
- /* MsgPack STR header is encoded in 5 bytes or less, followed by
- * the string data. */
- (void)str; return data + 5 + len;
-}
-
-static char *no_encode_bool(char *data, bool v)
-{
- /* MsgPack BOOL is encoded in 1 byte. */
- (void)v; return data + 1;
-}
-
-static char *no_encode_array_or_map(char *data, uint32_t len)
-{
- /* MsgPack ARRAY or MAP header is encoded in 5 bytes or less. */
- (void)len; return data + 5;
-}
-
-/*
- * If buf==NULL, return Enc that will perform size estimation;
- * otherwize, return Enc that renders results in the provided buf.
- */
-static const struct Enc *get_enc(void *buf)
-{
- static const struct Enc mp_enc = {
- mp_encode_uint, mp_encode_str, mp_encode_bool,
- mp_encode_array, mp_encode_map
- }, no_enc = {
- no_encode_uint, no_encode_str, no_encode_bool,
- no_encode_array_or_map, no_encode_array_or_map
- };
- return buf ? &mp_enc : &no_enc;
-}
-
-/*
* Convert SQLite affinity value to the corresponding Tarantool type
* string which is suitable for _index.parts field.
*/
@@ -1321,34 +1299,31 @@ static const char *convertSqliteAffinity(int affinity, bool allow_nulls)
}
}
-/*
- * Render "format" array for _space entry.
- * Returns result size.
- * If buf==NULL estimate result size.
- *
- * Ex: [{"name": "col1", "type": "integer"}, ... ]
- */
-int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf)
+char *
+sql_encode_table(struct region *region, struct Table *table, uint32_t *size)
{
- const struct Enc *enc = get_enc(buf);
- const struct space_def *def = pTable->def;
- assert(def != NULL);
- struct SqliteIndex *pk_idx = sqlite3PrimaryKeyIndex(pTable);
- int pk_forced_int = -1;
- char *base = buf, *p;
- int i, n = def->field_count;
-
- p = enc->encode_array(base, n);
+ size_t used = region_used(region);
+ struct mpstream stream;
+ bool is_error = false;
+ mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
+ set_encode_error, &is_error);
- /* If table's PK is single column which is INTEGER, then
- * treat it as strict type, not affinity. */
+ const struct space_def *def = table->def;
+ assert(def != NULL);
+ /*
+ * If table's PK is single column which is INTEGER, then
+ * treat it as strict type, not affinity.
+ */
+ struct SqliteIndex *pk_idx = sqlite3PrimaryKeyIndex(table);
+ uint32_t pk_forced_int = UINT32_MAX;
if (pk_idx != NULL && pk_idx->def->key_def->part_count == 1) {
int pk = pk_idx->def->key_def->parts[0].fieldno;
if (def->fields[pk].type == FIELD_TYPE_INTEGER)
pk_forced_int = pk;
}
-
- for (i = 0; i < n; i++) {
+ uint32_t field_count = def->field_count;
+ mpstream_encode_array(&stream, field_count);
+ for (uint32_t i = 0; i < field_count && !is_error; i++) {
const char *t;
uint32_t cid = def->fields[i].coll_id;
struct field_def *field = &def->fields[i];
@@ -1358,10 +1333,10 @@ int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf)
base_len += 1;
if (default_str != NULL)
base_len += 1;
- p = enc->encode_map(p, base_len);
- p = enc->encode_str(p, "name", 4);
- p = enc->encode_str(p, field->name, strlen(field->name));
- p = enc->encode_str(p, "type", 4);
+ mpstream_encode_map(&stream, base_len);
+ mpstream_encode_str(&stream, "name");
+ mpstream_encode_str(&stream, field->name);
+ mpstream_encode_str(&stream, "type");
if (i == pk_forced_int) {
t = "integer";
} else {
@@ -1370,114 +1345,145 @@ int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf)
convertSqliteAffinity(affinity,
def->fields[i].is_nullable);
}
-
assert(def->fields[i].is_nullable ==
- action_is_nullable(def->fields[i].nullable_action));
- p = enc->encode_str(p, t, strlen(t));
- p = enc->encode_str(p, "affinity", 8);
- p = enc->encode_uint(p, def->fields[i].affinity);
- p = enc->encode_str(p, "is_nullable", 11);
- p = enc->encode_bool(p, def->fields[i].is_nullable);
- p = enc->encode_str(p, "nullable_action", 15);
+ action_is_nullable(def->fields[i].nullable_action));
+ mpstream_encode_str(&stream, t);
+ mpstream_encode_str(&stream, "affinity");
+ mpstream_encode_uint(&stream, def->fields[i].affinity);
+ mpstream_encode_str(&stream, "is_nullable");
+ mpstream_encode_bool(&stream, def->fields[i].is_nullable);
+ mpstream_encode_str(&stream, "nullable_action");
assert(def->fields[i].nullable_action < on_conflict_action_MAX);
const char *action =
on_conflict_action_strs[def->fields[i].nullable_action];
- p = enc->encode_str(p, action, strlen(action));
+ mpstream_encode_str(&stream, action);
if (cid != COLL_NONE) {
- p = enc->encode_str(p, "collation", strlen("collation"));
- p = enc->encode_uint(p, cid);
+ mpstream_encode_str(&stream, "collation");
+ mpstream_encode_uint(&stream, cid);
}
if (default_str != NULL) {
- p = enc->encode_str(p, "default", strlen("default"));
- p = enc->encode_str(p, default_str, strlen(default_str));
+ mpstream_encode_str(&stream, "default");
+ mpstream_encode_str(&stream, default_str);
}
}
- return (int)(p - base);
+ mpstream_flush(&stream);
+ if (is_error) {
+ diag_set(OutOfMemory, stream.pos - stream.buf,
+ "mpstream_flush", "stream");
+ return NULL;
+ }
+ *size = region_used(region) - used;
+ char *raw = region_join(region, *size);
+ if (raw == NULL)
+ diag_set(OutOfMemory, *size, "region_join", "raw");
+ return raw;
}
-/*
- * Format "opts" dictionary for _space entry.
- * Returns result size.
- * If buf==NULL estimate result size.
- *
- * Ex: {"temporary": true, "sql": "CREATE TABLE student (name, grade)"}
- */
-int
-tarantoolSqlite3MakeTableOpts(Table *pTable, const char *zSql, char *buf)
+char *
+sql_encode_table_opts(struct region *region, struct Table *table,
+ const char *sql, uint32_t *size)
{
- const struct Enc *enc = get_enc(buf);
- bool is_view = pTable != NULL && pTable->def->opts.is_view;
- bool has_checks = pTable != NULL && pTable->def->opts.checks != NULL;
- int checks_cnt = has_checks ? pTable->def->opts.checks->nExpr : 0;
- char *p = enc->encode_map(buf, 1 + is_view + (checks_cnt > 0));
-
- p = enc->encode_str(p, "sql", 3);
- p = enc->encode_str(p, zSql, strlen(zSql));
+ assert(sql != NULL);
+ size_t used = region_used(region);
+ struct mpstream stream;
+ bool is_error = false;
+ mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
+ set_encode_error, &is_error);
+ bool is_view = false;
+ int checks_cnt = 0;
+ struct ExprList_item *a;
+ if (table != NULL) {
+ is_view = table->def->opts.is_view;
+ struct ExprList *checks = table->def->opts.checks;
+ if (checks != NULL) {
+ checks_cnt = checks->nExpr;
+ a = checks->a;
+ }
+ }
+ mpstream_encode_map(&stream, 1 + is_view + (checks_cnt > 0));
+
+ mpstream_encode_str(&stream, "sql");
+ mpstream_encode_str(&stream, sql);
if (is_view) {
- p = enc->encode_str(p, "view", 4);
- p = enc->encode_bool(p, true);
+ mpstream_encode_str(&stream, "view");
+ mpstream_encode_bool(&stream, true);
}
- if (checks_cnt == 0)
- return p - buf;
- /* Encode checks. */
- struct ExprList_item *a = pTable->def->opts.checks->a;
- p = enc->encode_str(p, "checks", 6);
- p = enc->encode_array(p, checks_cnt);
- for (int i = 0; i < checks_cnt; ++i) {
- int items = (a[i].pExpr != NULL) + (a[i].zName != NULL);
- p = enc->encode_map(p, items);
- /*
- * a[i].pExpr could be NULL for VIEW column names
- * represented as checks.
- */
- if (a[i].pExpr != NULL) {
- Expr *pExpr = a[i].pExpr;
- assert(pExpr->u.zToken != NULL);
- p = enc->encode_str(p, "expr", 4);
- p = enc->encode_str(p, pExpr->u.zToken,
- strlen(pExpr->u.zToken));
- }
- if (a[i].zName != NULL) {
- p = enc->encode_str(p, "name", 4);
- p = enc->encode_str(p, a[i].zName, strlen(a[i].zName));
+ if (checks_cnt > 0) {
+ mpstream_encode_str(&stream, "checks");
+ mpstream_encode_array(&stream, checks_cnt);
+ }
+ for (int i = 0; i < checks_cnt && !is_error; ++i, ++a) {
+ int items = (a->pExpr != NULL) + (a->zName != NULL);
+ mpstream_encode_map(&stream, items);
+ assert(a->pExpr != NULL);
+ struct Expr *pExpr = a->pExpr;
+ assert(pExpr->u.zToken != NULL);
+ mpstream_encode_str(&stream, "expr");
+ mpstream_encode_str(&stream, pExpr->u.zToken);
+ if (a->zName != NULL) {
+ mpstream_encode_str(&stream, "name");
+ mpstream_encode_str(&stream, a->zName);
}
}
- return p - buf;
+ mpstream_flush(&stream);
+ if (is_error) {
+ diag_set(OutOfMemory, stream.pos - stream.buf,
+ "mpstream_flush", "stream");
+ return NULL;
+ }
+ *size = region_used(region) - used;
+ char *raw = region_join(region, *size);
+ if (raw == NULL)
+ diag_set(OutOfMemory, *size, "region_join", "raw");
+ return raw;
}
-int
-fkey_encode_links(const struct fkey_def *def, int type, char *buf)
+char *
+fkey_encode_links(struct region *region, const struct fkey_def *def, int type,
+ uint32_t *size)
{
- const struct Enc *enc = get_enc(buf);
- char *p = enc->encode_array(buf, def->field_count);
- for (uint32_t i = 0; i < def->field_count; ++i)
- p = enc->encode_uint(p, def->links[i].fields[type]);
- return p - buf;
+ size_t used = region_used(region);
+ struct mpstream stream;
+ bool is_error = false;
+ mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
+ set_encode_error, &is_error);
+ uint32_t field_count = def->field_count;
+ mpstream_encode_array(&stream, field_count);
+ for (uint32_t i = 0; i < field_count && !is_error; ++i)
+ mpstream_encode_uint(&stream, def->links[i].fields[type]);
+ mpstream_flush(&stream);
+ if (is_error) {
+ diag_set(OutOfMemory, stream.pos - stream.buf,
+ "mpstream_flush", "stream");
+ return NULL;
+ }
+ *size = region_used(region) - used;
+ char *raw = region_join(region, *size);
+ if (raw == NULL)
+ diag_set(OutOfMemory, *size, "region_join", "raw");
+ return raw;
}
-/*
- * Format "parts" array for _index entry.
- * Returns result size.
- * If buf==NULL estimate result size.
- *
- * Ex: [[0, "integer"]]
- */
-int tarantoolSqlite3MakeIdxParts(SqliteIndex *pIndex, void *buf)
+char *
+sql_encode_index_parts(struct region *region, struct SqliteIndex *index,
+ uint32_t *size)
{
- struct field_def *fields = pIndex->pTable->def->fields;
- struct key_def *key_def = pIndex->def->key_def;
- const struct Enc *enc = get_enc(buf);
- char *base = buf;
+ size_t used = region_used(region);
+ struct mpstream stream;
+ bool is_error = false;
+ mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
+ set_encode_error, &is_error);
+ /*
+ * If table's PK is single column which is INTEGER, then
+ * treat it as strict type, not affinity.
+ */
uint32_t pk_forced_int = UINT32_MAX;
- struct SqliteIndex *primary_index =
- sqlite3PrimaryKeyIndex(pIndex->pTable);
-
- /* If table's PK is single column which is INTEGER, then
- * treat it as strict type, not affinity. */
- if (primary_index->def->key_def->part_count == 1) {
- int pk = primary_index->def->key_def->parts[0].fieldno;
- if (fields[pk].type == FIELD_TYPE_INTEGER)
- pk_forced_int = pk;
+ struct SqliteIndex *pk = sqlite3PrimaryKeyIndex(index->pTable);
+ struct field_def *fields = index->pTable->def->fields;
+ if (pk->def->key_def->part_count == 1) {
+ int fieldno = pk->def->key_def->parts[0].fieldno;
+ if (fields[fieldno].type == FIELD_TYPE_INTEGER)
+ pk_forced_int = fieldno;
}
/* gh-2187
@@ -1486,8 +1492,9 @@ int tarantoolSqlite3MakeIdxParts(SqliteIndex *pIndex, void *buf)
* primary key columns. Query planner depends on this particular
* data layout.
*/
+ struct key_def *key_def = index->def->key_def;
struct key_part *part = key_def->parts;
- char *p = enc->encode_array(base, key_def->part_count);
+ mpstream_encode_array(&stream, key_def->part_count);
for (uint32_t i = 0; i < key_def->part_count; ++i, ++part) {
uint32_t col = part->fieldno;
assert(fields[col].is_nullable ==
@@ -1499,54 +1506,64 @@ int tarantoolSqlite3MakeIdxParts(SqliteIndex *pIndex, void *buf)
t = convertSqliteAffinity(fields[col].affinity,
fields[col].is_nullable);
}
- /* do not decode default collation */
+ /* Do not decode default collation. */
uint32_t cid = part->coll_id;
- p = enc->encode_map(p, cid == COLL_NONE ? 5 : 6);
- p = enc->encode_str(p, "type", sizeof("type")-1);
- p = enc->encode_str(p, t, strlen(t));
- p = enc->encode_str(p, "field", sizeof("field")-1);
- p = enc->encode_uint(p, col);
+ mpstream_encode_map(&stream, 5 + (cid != COLL_NONE));
+ mpstream_encode_str(&stream, "type");
+ mpstream_encode_str(&stream, t);
+ mpstream_encode_str(&stream, "field");
+ mpstream_encode_uint(&stream, col);
if (cid != COLL_NONE) {
- p = enc->encode_str(p, "collation",
- sizeof("collation") - 1);
- p = enc->encode_uint(p, cid);
+ mpstream_encode_str(&stream, "collation");
+ mpstream_encode_uint(&stream, cid);
}
- p = enc->encode_str(p, "is_nullable", 11);
- p = enc->encode_bool(p, fields[col].is_nullable);
- p = enc->encode_str(p, "nullable_action", 15);
+ mpstream_encode_str(&stream, "is_nullable");
+ mpstream_encode_bool(&stream, fields[col].is_nullable);
+ mpstream_encode_str(&stream, "nullable_action");
const char *action_str =
on_conflict_action_strs[fields[col].nullable_action];
- p = enc->encode_str(p, action_str, strlen(action_str));
+ mpstream_encode_str(&stream, action_str);
- p = enc->encode_str(p, "sort_order", 10);
+ mpstream_encode_str(&stream, "sort_order");
enum sort_order sort_order = part->sort_order;
assert(sort_order < sort_order_MAX);
const char *sort_order_str = sort_order_strs[sort_order];
- p = enc->encode_str(p, sort_order_str, strlen(sort_order_str));
+ mpstream_encode_str(&stream, sort_order_str);
}
- return p - base;
+ mpstream_flush(&stream);
+ if (is_error) {
+ diag_set(OutOfMemory, stream.pos - stream.buf,
+ "mpstream_flush", "stream");
+ return NULL;
+ }
+ *size = region_used(region) - used;
+ char *raw = region_join(region, *size);
+ if (raw == NULL)
+ diag_set(OutOfMemory, *size, "region_join", "raw");
+ return raw;
}
-int
-sql_encode_index_opts(struct index_opts *opts, void *buf)
+char *
+sql_encode_index_opts(struct region *region, struct index_opts *opts,
+ uint32_t *size)
{
- const struct Enc *enc = get_enc(buf);
- char *base = buf, *p;
-
- p = enc->encode_map(base, 2);
- /* Mark as unique pk and unique indexes */
- p = enc->encode_str(p, "unique", 6);
- /* If user didn't defined ON CONFLICT OPTIONS, all uniqueness checks
- * will be made by Tarantool. However, Tarantool doesn't have ON
- * CONFLIT option, so in that case (except ON CONFLICT ABORT, which is
- * default behavior) uniqueness will be checked by SQL.
- * INSERT OR REPLACE/IGNORE uniqueness checks will be also done by
- * Tarantool.
- */
- p = enc->encode_bool(p, opts->is_unique);
- p = enc->encode_str(p, "sql", 3);
- p = enc->encode_str(p, opts->sql, opts->sql ? strlen(opts->sql) : 0);
- return (int)(p - base);
+ size_t used = region_used(region);
+ struct mpstream stream;
+ bool is_error = false;
+ mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
+ set_encode_error, &is_error);
+ mpstream_encode_index_opts(&stream, opts);
+ mpstream_flush(&stream);
+ if (is_error) {
+ diag_set(OutOfMemory, stream.pos - stream.buf, "mpstream_flush",
+ "stream");
+ return NULL;
+ }
+ *size = region_used(region) - used;
+ char *raw = region_join(region, *size);
+ if (raw == NULL)
+ diag_set(OutOfMemory, *size, "region_join", "raw");
+ return raw;
}
void
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index da7668b..c422a66 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1241,26 +1241,29 @@ createIndex(Parse * pParse, Index * pIndex, int iSpaceId, int iIndexId,
Vdbe *v = sqlite3GetVdbe(pParse);
int iFirstCol = ++pParse->nMem;
int iRecord = (pParse->nMem += 6); /* 6 total columns */
- char *zOpts, *zParts;
- int zOptsSz, zPartsSz;
-
- /* Format "opts" and "parts" for _index entry. */
struct index_opts opts = pIndex->def->opts;
opts.sql = (char *)zSql;
- zOpts = sqlite3DbMallocRaw(pParse->db,
- sql_encode_index_opts(&opts, NULL) +
- tarantoolSqlite3MakeIdxParts(pIndex,
- NULL) + 2);
- if (!zOpts)
+ struct region *region = &pParse->region;
+ uint32_t index_opts_sz = 0;
+ char *index_opts =
+ sql_encode_index_opts(region, &opts, &index_opts_sz);
+ if (index_opts == NULL)
+ goto error;
+ uint32_t index_parts_sz = 0;
+ char *index_parts =
+ sql_encode_index_parts(region, pIndex, &index_parts_sz);
+ if (index_parts == NULL)
+ goto error;
+ char *raw = sqlite3DbMallocRaw(pParse->db,
+ index_opts_sz + index_parts_sz);
+ if (raw == NULL)
return;
- zOptsSz = sql_encode_index_opts(&opts, zOpts);
- zParts = zOpts + zOptsSz + 1;
- zPartsSz = tarantoolSqlite3MakeIdxParts(pIndex, zParts);
-#if SQLITE_DEBUG
- /* NUL-termination is necessary for VDBE trace facility only */
- zOpts[zOptsSz] = 0;
- zParts[zPartsSz] = 0;
-#endif
+
+ memcpy(raw, index_opts, index_opts_sz);
+ index_opts = raw;
+ raw += index_opts_sz;
+ memcpy(raw, index_parts, index_parts_sz);
+ index_parts = raw;
if (pParse->pNewTable) {
int reg;
@@ -1296,16 +1299,20 @@ createIndex(Parse * pParse, Index * pIndex, int iSpaceId, int iIndexId,
P4_DYNAMIC);
sqlite3VdbeAddOp4(v, OP_String8, 0, iFirstCol + 3, 0, "tree",
P4_STATIC);
- sqlite3VdbeAddOp4(v, OP_Blob, zOptsSz, iFirstCol + 4,
- SQL_SUBTYPE_MSGPACK, zOpts, P4_DYNAMIC);
+ sqlite3VdbeAddOp4(v, OP_Blob, index_opts_sz, iFirstCol + 4,
+ SQL_SUBTYPE_MSGPACK, index_opts, P4_DYNAMIC);
/* zOpts and zParts are co-located, hence STATIC */
- sqlite3VdbeAddOp4(v, OP_Blob, zPartsSz, iFirstCol + 5,
- SQL_SUBTYPE_MSGPACK,zParts, P4_STATIC);
+ sqlite3VdbeAddOp4(v, OP_Blob, index_parts_sz, iFirstCol + 5,
+ SQL_SUBTYPE_MSGPACK, index_parts, P4_STATIC);
sqlite3VdbeAddOp3(v, OP_MakeRecord, iFirstCol, 6, iRecord);
sqlite3VdbeAddOp2(v, OP_SInsert, BOX_INDEX_ID, iRecord);
if (pIndex->index_type == SQL_INDEX_TYPE_NON_UNIQUE ||
pIndex->index_type == SQL_INDEX_TYPE_UNIQUE)
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+ return;
+error:
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
}
/*
@@ -1360,50 +1367,54 @@ makeIndexSchemaRecord(Parse * pParse,
static void
createSpace(Parse * pParse, int iSpaceId, char *zStmt)
{
- Table *p = pParse->pNewTable;
+ struct Table *table = pParse->pNewTable;
Vdbe *v = sqlite3GetVdbe(pParse);
int iFirstCol = ++pParse->nMem;
int iRecord = (pParse->nMem += 7);
- char *zOpts, *zFormat;
- int zOptsSz, zFormatSz;
-
- zOpts = sqlite3DbMallocRaw(pParse->db,
- tarantoolSqlite3MakeTableFormat(p, NULL) +
- tarantoolSqlite3MakeTableOpts(p, zStmt,
- NULL) + 2);
- if (!zOpts) {
- zOptsSz = 0;
- zFormat = NULL;
- zFormatSz = 0;
- } else {
- zOptsSz = tarantoolSqlite3MakeTableOpts(p, zStmt, zOpts);
- zFormat = zOpts + zOptsSz + 1;
- zFormatSz = tarantoolSqlite3MakeTableFormat(p, zFormat);
-#if SQLITE_DEBUG
- /* NUL-termination is necessary for VDBE-tracing facility only */
- zOpts[zOptsSz] = 0;
- zFormat[zFormatSz] = 0;
-#endif
- }
+ struct region *region = &pParse->region;
+ uint32_t table_opts_stmt_sz = 0;
+ char *table_opts_stmt = sql_encode_table_opts(region, table, zStmt,
+ &table_opts_stmt_sz);
+ if (table_opts_stmt == NULL)
+ goto error;
+ uint32_t table_stmt_sz = 0;
+ char *table_stmt = sql_encode_table(region, table, &table_stmt_sz);
+ if (table_stmt == NULL)
+ goto error;
+ char *raw = sqlite3DbMallocRaw(pParse->db,
+ table_stmt_sz + table_opts_stmt_sz);
+ if (raw == NULL)
+ return;
+
+ memcpy(raw, table_opts_stmt, table_opts_stmt_sz);
+ table_opts_stmt = raw;
+ raw += table_opts_stmt_sz;
+ memcpy(raw, table_stmt, table_stmt_sz);
+ table_stmt = raw;
sqlite3VdbeAddOp2(v, OP_SCopy, iSpaceId, iFirstCol /* spaceId */ );
sqlite3VdbeAddOp2(v, OP_Integer, effective_user()->uid,
iFirstCol + 1 /* owner */ );
sqlite3VdbeAddOp4(v, OP_String8, 0, iFirstCol + 2 /* name */ , 0,
- sqlite3DbStrDup(pParse->db, p->def->name), P4_DYNAMIC);
+ sqlite3DbStrDup(pParse->db, table->def->name),
+ P4_DYNAMIC);
sqlite3VdbeAddOp4(v, OP_String8, 0, iFirstCol + 3 /* engine */ , 0,
- sqlite3DbStrDup(pParse->db, p->def->engine_name),
+ sqlite3DbStrDup(pParse->db, table->def->engine_name),
P4_DYNAMIC);
- sqlite3VdbeAddOp2(v, OP_Integer, p->def->field_count,
+ sqlite3VdbeAddOp2(v, OP_Integer, table->def->field_count,
iFirstCol + 4 /* field_count */ );
- sqlite3VdbeAddOp4(v, OP_Blob, zOptsSz, iFirstCol + 5,
- SQL_SUBTYPE_MSGPACK, zOpts, P4_DYNAMIC);
+ sqlite3VdbeAddOp4(v, OP_Blob, table_opts_stmt_sz, iFirstCol + 5,
+ SQL_SUBTYPE_MSGPACK, table_opts_stmt, P4_DYNAMIC);
/* zOpts and zFormat are co-located, hence STATIC */
- sqlite3VdbeAddOp4(v, OP_Blob, zFormatSz, iFirstCol + 6,
- SQL_SUBTYPE_MSGPACK, zFormat, P4_STATIC);
+ sqlite3VdbeAddOp4(v, OP_Blob, table_stmt_sz, iFirstCol + 6,
+ SQL_SUBTYPE_MSGPACK, table_stmt, P4_STATIC);
sqlite3VdbeAddOp3(v, OP_MakeRecord, iFirstCol, 7, iRecord);
sqlite3VdbeAddOp2(v, OP_SInsert, BOX_SPACE_ID, iRecord);
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+ return;
+error:
+ pParse->nErr++;
+ pParse->rc = SQL_TARANTOOL_ERROR;
}
/*
@@ -1592,10 +1603,8 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk)
BOX_FK_CONSTRAINT_ID, 0,
constr_tuple_reg, 2,
ER_CONSTRAINT_EXISTS, error_msg,
- false, OP_NoConflict) != 0) {
- sqlite3DbFree(parse_context->db, name_copy);
+ false, OP_NoConflict) != 0)
return;
- }
sqlite3VdbeAddOp2(vdbe, OP_Bool, 0, constr_tuple_reg + 3);
sqlite3VdbeChangeP4(vdbe, -1, (char*)&fk->is_deferred, P4_BOOL);
sqlite3VdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 4, 0,
@@ -1604,33 +1613,48 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk)
fkey_action_strs[fk->on_delete], P4_STATIC);
sqlite3VdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 6, 0,
fkey_action_strs[fk->on_update], P4_STATIC);
- size_t parent_size = fkey_encode_links(fk, FIELD_LINK_PARENT, NULL);
- size_t child_size = fkey_encode_links(fk, FIELD_LINK_CHILD, NULL);
+ struct region *region = &parse_context->region;
+ uint32_t parent_links_size = 0;
+ char *parent_links = fkey_encode_links(region, fk, FIELD_LINK_PARENT,
+ &parent_links_size);
+ if (parent_links == NULL)
+ goto error;
+ uint32_t child_links_size = 0;
+ char *child_links = fkey_encode_links(region, fk, FIELD_LINK_CHILD,
+ &child_links_size);
+ if (child_links == NULL)
+ goto error;
/*
* We are allocating memory for both parent and child
* arrays in the same chunk. Thus, first OP_Blob opcode
* interprets it as static memory, and the second one -
* as dynamic and releases memory.
*/
- char *parent_links = sqlite3DbMallocRaw(parse_context->db,
- parent_size + child_size);
- if (parent_links == NULL) {
- sqlite3DbFree(parse_context->db, (void *) name_copy);
+ char *raw = sqlite3DbMallocRaw(parse_context->db,
+ parent_links_size + child_links_size);
+ if (raw == NULL)
return;
- }
- parent_size = fkey_encode_links(fk, FIELD_LINK_PARENT, parent_links);
- char *child_links = parent_links + parent_size;
- child_size = fkey_encode_links(fk, FIELD_LINK_CHILD, child_links);
- sqlite3VdbeAddOp4(vdbe, OP_Blob, child_size, constr_tuple_reg + 7,
+ memcpy(raw, parent_links, parent_links_size);
+ parent_links = raw;
+ raw += parent_links_size;
+ memcpy(raw, child_links, child_links_size);
+ child_links = raw;
+
+ sqlite3VdbeAddOp4(vdbe, OP_Blob, child_links_size, constr_tuple_reg + 7,
SQL_SUBTYPE_MSGPACK, child_links, P4_STATIC);
- sqlite3VdbeAddOp4(vdbe, OP_Blob, parent_size, constr_tuple_reg + 8,
- SQL_SUBTYPE_MSGPACK, parent_links, P4_DYNAMIC);
+ sqlite3VdbeAddOp4(vdbe, OP_Blob, parent_links_size,
+ constr_tuple_reg + 8, SQL_SUBTYPE_MSGPACK,
+ parent_links, P4_DYNAMIC);
sqlite3VdbeAddOp3(vdbe, OP_MakeRecord, constr_tuple_reg, 9,
constr_tuple_reg + 9);
sqlite3VdbeAddOp2(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID,
constr_tuple_reg + 9);
sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE);
sqlite3ReleaseTempRange(parse_context, constr_tuple_reg, 10);
+ return;
+error:
+ parse_context->nErr++;
+ parse_context->rc = SQL_TARANTOOL_ERROR;
}
/**
diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index a3ad52a..07372c1 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -151,49 +151,73 @@ tarantoolSqlite3IdxKeyCompare(struct BtCursor *cursor,
int
tarantoolSqlite3IncrementMaxid(uint64_t *space_max_id);
-/*
- * Render "format" array for _space entry.
- * Returns result size.
- * If buf==NULL estimate result size.
+/**
+ * Encode format as entry to be inserted to _space on @region.
+ * @param region Region to use.
+ * @param table Table to encode.
+ * @param[out] size Size of result allocation.
+ *
+ * @retval NULL Error.
+ * @retval not NULL Pointer to msgpack on success.
*/
-int tarantoolSqlite3MakeTableFormat(Table * pTable, void *buf);
+char *
+sql_encode_table(struct region *region, struct Table *table, uint32_t *size);
-/*
- * Format "opts" dictionary for _space entry.
- * Returns result size.
- * If buf==NULL estimate result size.
+/**
+ * Encode "opts" dictionary for _space entry on @region.
+ * @param region Region to use.
+ * @param table Table containing opts to encode.
+ * @param sql Source request to encode.
+ * @param[out] size Size of result allocation.
+ *
+ * @retval NULL Error.
+ * @retval not NULL Pointer to msgpack on success.
*/
-int tarantoolSqlite3MakeTableOpts(Table * pTable, const char *zSql, char *buf);
+char *
+sql_encode_table_opts(struct region *region, struct Table *table,
+ const char *sql, uint32_t *size);
/**
- * Encode links of given foreign key constraint into MsgPack.
- *
+ * Encode links of given foreign key constraint into MsgPack on
+ * @region.
+ * @param region Wegion to use.
* @param def FK def to encode links of.
* @param type Links type to encode.
- * @param buf Buffer to hold encoded links. Can be NULL. In this
- * case function would simply calculate memory required
- * for such buffer.
- * @retval Length of encoded array.
+ * @param[out] Size size of result allocation.
+ *
+ * @retval NULL Error.
+ * @retval not NULL Pointer to msgpack on success.
*/
-int
-fkey_encode_links(const struct fkey_def *def, int type, char *buf);
+char *
+fkey_encode_links(struct region *region, const struct fkey_def *def, int type,
+ uint32_t *size);
-/*
- * Format "parts" array for _index entry.
- * Returns result size.
- * If buf==NULL estimate result size.
+/**
+ * Encode index parts of given foreign key constraint into
+ * MsgPack on @region.
+ * @param region Region to use.
+ * @param index Index to encode.
+ * @param[out] size Size of result allocation.
+ *
+ * @retval NULL Error.
+ * @retval not NULL Pointer to msgpack on success
*/
-int tarantoolSqlite3MakeIdxParts(Index * index, void *buf);
+char *
+sql_encode_index_parts(struct region *region, struct Index *index,
+ uint32_t *size);
/**
- * Format "opts" dictionary for _index entry.
+ * Encode "opts" dictionary for _index entry on @region.
*
+ * @param region region to use.
* @param opts Options to encode.
- * @param[out] buf destination buffer.
- * @retval Result size or size estimation if buf == NULL.
+ * @param[out] size size of result allocation.
+ * @retval NULL on error.
+ * @retval not NULL pointer to msgpack on success
*/
-int
-sql_encode_index_opts(struct index_opts *opts, void *buf);
+char *
+sql_encode_index_opts(struct region *region, struct index_opts *opts,
+ uint32_t *size);
/**
* Extract next id from _sequence space.
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index bd730c4..f400c25 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -173,16 +173,12 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list,
{
/* Trigger being finished. */
struct sql_trigger *trigger = parse->parsed_ast.trigger;
- /* SQL text. */
- char *sql_str = NULL;
- /* MsgPack containing SQL options. */
- char *opts_buff = NULL;
/* The database. */
struct sqlite3 *db = parse->db;
parse->parsed_ast.trigger = NULL;
if (NEVER(parse->nErr) || trigger == NULL)
- goto triggerfinish_cleanup;
+ goto cleanup;
char *trigger_name = trigger->zName;
trigger->step_list = step_list;
while (step_list != NULL) {
@@ -202,17 +198,18 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list,
/* Make an entry in the _trigger space. */
struct Vdbe *v = sqlite3GetVdbe(parse);
if (v == 0)
- goto triggerfinish_cleanup;
+ goto cleanup;
struct Table *sys_trigger =
- sqlite3HashFind(&parse->db->pSchema->tblHash,
+ sqlite3HashFind(&db->pSchema->tblHash,
TARANTOOL_SYS_TRIGGER_NAME);
if (NEVER(sys_trigger == NULL))
- goto triggerfinish_cleanup;
+ goto cleanup;
- sql_str = sqlite3MPrintf(db, "CREATE TRIGGER %s", token->z);
- if (db->mallocFailed)
- goto triggerfinish_cleanup;
+ char *sql_str =
+ sqlite3MPrintf(db, "CREATE TRIGGER %s", token->z);
+ if (sql_str == NULL)
+ goto cleanup;
int cursor = parse->nTab++;
sqlite3OpenTable(parse, cursor, sys_trigger, OP_OpenWrite);
@@ -225,24 +222,27 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list,
parse->nMem += 3;
int record = ++parse->nMem;
- opts_buff =
- sqlite3DbMallocRaw(parse->db,
- tarantoolSqlite3MakeTableOpts(0,
- sql_str,
- NULL) +
- 1);
- if (db->mallocFailed)
- goto triggerfinish_cleanup;
-
- int opts_buff_sz =
- tarantoolSqlite3MakeTableOpts(0, sql_str, opts_buff);
-
- trigger_name = sqlite3DbStrDup(parse->db, trigger_name);
- if (db->mallocFailed)
- goto triggerfinish_cleanup;
+ uint32_t opts_buff_sz = 0;
+ char *data = sql_encode_table_opts(&fiber()->gc, NULL, sql_str,
+ &opts_buff_sz);
+ sqlite3DbFree(db, sql_str);
+ if (data == NULL) {
+ parse->nErr++;
+ parse->rc = SQL_TARANTOOL_ERROR;
+ goto cleanup;
+ }
+ char *opts_buff = sqlite3DbMallocRaw(db, opts_buff_sz);
+ if (opts_buff == NULL)
+ goto cleanup;
+ memcpy(opts_buff, data, opts_buff_sz);
+
+ trigger_name = sqlite3DbStrDup(db, trigger_name);
+ if (trigger_name == NULL) {
+ sqlite3DbFree(db, opts_buff);
+ goto cleanup;
+ }
- sqlite3VdbeAddOp4(v,
- OP_String8, 0, first_col, 0,
+ sqlite3VdbeAddOp4(v, OP_String8, 0, first_col, 0,
trigger_name, P4_DYNAMIC);
sqlite3VdbeAddOp2(v, OP_Integer, trigger->space_id,
first_col + 1);
@@ -261,11 +261,7 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list,
trigger = NULL;
}
- triggerfinish_cleanup:
- if (db->mallocFailed) {
- sqlite3DbFree(db, sql_str);
- sqlite3DbFree(db, opts_buff);
- }
+cleanup:
sql_trigger_delete(db, trigger);
assert(parse->parsed_ast.trigger == NULL || parse->parse_only);
sqlite3DeleteTriggerStep(db, step_list);
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] sql: remove struct Enc
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 2/2] sql: remove struct Enc Kirill Shcherbatov
@ 2018-08-28 1:43 ` Vladislav Shpilevoy
2018-08-28 6:46 ` Kirill Shcherbatov
0 siblings, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2018-08-28 1:43 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
Thanks for the patch! See 7 comments below and a
commit on the branch.
On 27/08/2018 08:11, Kirill Shcherbatov wrote:
> We shouldn't use legacy Enc structure in SQL as we have
> featured class mpstream in tarantool core. With this path
> all allocations temporally commited with mpstream initialized
> on region and then duplicated in dynamic persistent memory via
> sqlite3DbMallocRaw.
>
> Closes #3545.
> ---
> src/box/sql.c | 473 +++++++++++++++++++++++----------------------
> src/box/sql/build.c | 156 ++++++++-------
> src/box/sql/tarantoolInt.h | 80 +++++---
> src/box/sql/trigger.c | 62 +++---
> 4 files changed, 416 insertions(+), 355 deletions(-)
>
> diff --git a/src/box/sql.c b/src/box/sql.c
> index b98593b..6bf832b 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -56,6 +56,8 @@
> #include "xrow.h"
> #include "iproto_constants.h"
> #include "fkey.h"
> +#include "mpstream.h"
> +#include "small/region.h"
1. Double include of region.h.
>
> static sqlite3 *db = NULL;
>
> @@ -840,6 +842,32 @@ rename_fail:
> return SQL_TARANTOOL_ERROR;
> }
>
> +/** Callback to forward and error from mpstream methods. */
> +static void
> +set_encode_error(void *error_ctx)
> +{
> + *(bool *)error_ctx = true;
> +}
> +
> +void
> +mpstream_encode_index_opts(struct mpstream *stream, struct index_opts *opts)
2. Static.
3. No comment.
> +{
> + mpstream_encode_map(stream, 2);
> + /* Mark as unique pk and unique indexes */
4. Finish a sentence with the dot. Btw the comment
is not related to the code. Below you do not whether
the index is unique or not.
> + mpstream_encode_str(stream, "unique");
> + /* If user didn't defined ON CONFLICT OPTIONS, all uniqueness checks
> + * will be made by Tarantool. However, Tarantool doesn't have ON
> + * CONFLIT option, so in that case (except ON CONFLICT ABORT, which is
> + * default behavior) uniqueness will be checked by SQL.
> + * INSERT OR REPLACE/IGNORE uniqueness checks will be also done by
> + * Tarantool.
> + */
5. "Did't defined" - corrupted phrase. Rather "did't define".
But the whole comment looks strange as well as the previous,
and out of its place. Just remove it.
> + mpstream_encode_bool(stream, opts->is_unique);
> + mpstream_encode_str(stream, "sql");
> + mpstream_encode_strn(stream, opts->sql,
> + opts->sql != NULL ? strlen(opts->sql) : 0);> +}
> +
> int
> sql_index_update_table_name(struct index_def *def, const char *new_tbl_name,
> char **sql_stmt)
> @@ -851,33 +879,43 @@ sql_index_update_table_name(struct index_def *def, const char *new_tbl_name,
> if (*sql_stmt == NULL)
> return -1;
>
> - uint32_t key_len = mp_sizeof_uint(def->space_id) +
> - mp_sizeof_uint(def->iid) + mp_sizeof_array(2);
> + struct region *region = &fiber()->gc;
> + size_t used = region_used(region);
> + struct mpstream stream;
> + bool is_error = false;
> + mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
> + set_encode_error, &is_error);
> +
> + /* Encode key. */
> + mpstream_encode_array(&stream, 2);
> + mpstream_encode_uint(&stream, def->space_id);
> + mpstream_encode_uint(&stream, def->iid);
> +
> + /* Encode op. */
> + uint32_t op_offset = stream.pos - stream.buf;
> + mpstream_encode_array(&stream, 1);
> + mpstream_encode_array(&stream, 3);
> + mpstream_encode_str(&stream, "=");
> + mpstream_encode_uint(&stream, BOX_INDEX_FIELD_OPTS);
> +
> + /* Encode index opts. */
> struct index_opts opts = def->opts;
> opts.sql = *sql_stmt;
> - uint32_t new_opts_sz = sql_encode_index_opts(&opts, NULL);
> - uint32_t op_sz = mp_sizeof_array(1) + mp_sizeof_array(3) +
> - mp_sizeof_str(1) + mp_sizeof_uint(4) + new_opts_sz;
> + mpstream_encode_index_opts(&stream, &opts);
>
> - char *key_begin = (char*) region_alloc(&fiber()->gc, key_len + op_sz);
> - if (key_begin == NULL) {
> - diag_set(OutOfMemory, key_len + op_sz, "region_alloc",
> - "key_begin");
> + mpstream_flush(&stream);
> + if (is_error) {
> + diag_set(OutOfMemory, stream.pos - stream.buf,
> + "mpstream_flush", "stream");
> return -1;
> }
> - char *key = mp_encode_array(key_begin, 2);
> - key = mp_encode_uint(key, def->space_id);
> - key = mp_encode_uint(key, def->iid);
> -
> - char *op_begin = key;
> - char *op = mp_encode_array(op_begin, 1);
> - op = mp_encode_array(op, 3);
> - op = mp_encode_str(op, "=", 1);
> - op = mp_encode_uint(op, BOX_INDEX_FIELD_OPTS);
> - op += sql_encode_index_opts(&opts, op);
> -
> - if (box_update(BOX_INDEX_ID, 0, key_begin, key, op_begin, op,
> - 0, NULL) != 0)
> + size_t sz = region_used(region) - used;
> + char *raw = region_join(region, sz);
> + if (raw == NULL)
> + diag_set(OutOfMemory, sz, "region_join", "raw");
> +
> + if (box_update(BOX_INDEX_ID, 0, raw, raw + op_offset, raw + op_offset,
6. If 'raw == NULL' you still try to use it in box_update.
> + raw + sz, 0, NULL) != 0)
> return -1;
>
> return 0;
7. Here https://github.com/tarantool/tarantool/commit/0d8b0c5641ec4c27547b1d0121bb1e0a641b149b
I see changes not relevant to this patch and not presented in the
email. I paste them bellow:
> diff --git a/src/mpstream.c b/src/mpstream.c
> index 23d27f9e5..44c2d9213 100644
> --- a/src/mpstream.c
> +++ b/src/mpstream.c
> @@ -176,12 +176,6 @@ mpstream_encode_strn(struct mpstream *stream, const char *str, uint32_t len)
> mpstream_advance(stream, pos - data);
> }
>
> -void
> -mpstream_encode_str(struct mpstream *stream, const char *str)
> -{
> - mpstream_encode_strn(stream, str, strlen(str));
> -}
> -
> void
> mpstream_encode_nil(struct mpstream *stream)
> {
> diff --git a/src/mpstream.h b/src/mpstream.h
> index 789bd741d..6f9462d4c 100644
> --- a/src/mpstream.h
> +++ b/src/mpstream.h
> @@ -108,8 +108,11 @@ mpstream_encode_double(struct mpstream *stream, double num);
> void
> mpstream_encode_strn(struct mpstream *stream, const char *str, uint32_t len);
>
> -void
> -mpstream_encode_str(struct mpstream *stream, const char *str);
> +static inline void
> +mpstream_encode_str(struct mpstream *stream, const char *str)
> +{
> + mpstream_encode_strn(stream, str, strlen(str));
> +}
>
> void
> mpstream_encode_nil(struct mpstream *stream);
Obviously, it should not be part of this comment, but
of the previous one.
Below and on the branch you can find my fixes:
diff --git a/src/box/sql.c b/src/box/sql.c
index 6bf832bd7..d1b935c14 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -57,7 +57,6 @@
#include "iproto_constants.h"
#include "fkey.h"
#include "mpstream.h"
-#include "small/region.h"
static sqlite3 *db = NULL;
@@ -911,14 +910,12 @@ sql_index_update_table_name(struct index_def *def, const char *new_tbl_name,
}
size_t sz = region_used(region) - used;
char *raw = region_join(region, sz);
- if (raw == NULL)
+ if (raw == NULL) {
diag_set(OutOfMemory, sz, "region_join", "raw");
-
- if (box_update(BOX_INDEX_ID, 0, raw, raw + op_offset, raw + op_offset,
- raw + sz, 0, NULL) != 0)
return -1;
-
- return 0;
+ }
+ return box_update(BOX_INDEX_ID, 0, raw, raw + op_offset,
+ raw + op_offset, raw + sz, 0, NULL);
}
int
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/2] box: export mpstream methods to core
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 1/2] box: export mpstream methods to core Kirill Shcherbatov
@ 2018-08-28 1:43 ` Vladislav Shpilevoy
2018-08-28 6:46 ` Kirill Shcherbatov
0 siblings, 1 reply; 10+ messages in thread
From: Vladislav Shpilevoy @ 2018-08-28 1:43 UTC (permalink / raw)
To: tarantool-patches, Kirill Shcherbatov
Thanks for the patch! See 2 comments below.
On 27/08/2018 08:11, Kirill Shcherbatov wrote:
> As we going to use mpstream not only in LUA, let's move this
> API in tarantool core.
>
> Part of #3545.
> ---
> src/CMakeLists.txt | 1 +
> src/box/lua/call.c | 11 +--
> src/box/lua/misc.cc | 1 +
> src/box/lua/net_box.c | 127 +++++++++++++++----------------
> src/box/lua/tuple.c | 23 +++---
> src/lua/msgpack.c | 166 ++++------------------------------------
> src/lua/msgpack.h | 102 +------------------------
> src/mpstream.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> src/mpstream.h | 124 ++++++++++++++++++++++++++++++
> 9 files changed, 429 insertions(+), 331 deletions(-)
> create mode 100644 src/mpstream.c
> create mode 100644 src/mpstream.h
> > diff --git a/src/mpstream.c b/src/mpstream.c
> new file mode 100644
> index 0000000..23d27f9
> --- /dev/null
> +++ b/src/mpstream.c
> @@ -0,0 +1,205 @@
> +/*
> + * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above
> + * copyright notice, this list of conditions and the
> + * following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer in the documentation and/or other materials
> + * provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#include "mpstream.h"
> +#include <assert.h>
> +#include <stdint.h>
> +#include "msgpuck.h"
> +
> +void
> +mpstream_reserve_slow(struct mpstream *stream, size_t size)
> +{
> + stream->alloc(stream->ctx, stream->pos - stream->buf);
> + stream->buf = (char *) stream->reserve(stream->ctx, &size);
> + if (stream->buf == NULL) {
> + diag_set(OutOfMemory, size, "mpstream", "reserve");
> + stream->error(stream->error_ctx);
> + }
> + stream->pos = stream->buf;
> + stream->end = stream->pos + size;
> +}
> +
> +void
> +mpstream_reset(struct mpstream *stream)
> +{
> + size_t size = 0;
> + stream->buf = (char *) stream->reserve(stream->ctx, &size);
> + if (stream->buf == NULL) {
> + diag_set(OutOfMemory, size, "mpstream", "reset");
> + stream->error(stream->error_ctx);
> + }
> + stream->pos = stream->buf;
> + stream->end = stream->pos + size;
> +}
> +
> +/**
> + * A streaming API so that it's possible to encode to any output
> + * stream.
> + */
> +void
> +mpstream_init(struct mpstream *stream, void *ctx,
> + mpstream_reserve_f reserve, mpstream_alloc_f alloc,
> + mpstream_error_f error, void *error_ctx)
> +{
> + stream->ctx = ctx;
> + stream->reserve = reserve;
> + stream->alloc = alloc;
> + stream->error = error;
> + stream->error_ctx = error_ctx;
> + mpstream_reset(stream);
> +}
> +
> +void
> +mpstream_flush(struct mpstream *stream)
> +{
> + stream->alloc(stream->ctx, stream->pos - stream->buf);
> + stream->buf = stream->pos;
> +}
> +
> +char *
> +mpstream_reserve(struct mpstream *stream, size_t size)
> +{
> + if (stream->pos + size > stream->end)
> + mpstream_reserve_slow(stream, size);
> + return stream->pos;
> +}
> +
> +void
> +mpstream_advance(struct mpstream *stream, size_t size)
> +{
> + assert(stream->pos + size <= stream->end);
> + stream->pos += size;
> +}
1. As I said in the previous review, reserve and
reserve_slow are not split in two functions just
for case. There is a reason. And your patch
destroys the benefit of reserve/reserve_slow
splitting. I paste it here:
"There was a reason, why reserve_slow is an independent function
and reserve is static inline. This is why in most cases reserve
just returns a current position, and rarely it falls down to
alloc(). Please, return as it was. Same for advance."
Keywords:
*"return as it was",
* "reserve_slow is independent, reserve is static inline",
* "same for advance".
Static inline reserve in a header provides a benefit of
inlining in most cases and rarely falls down to reserve_slow,
that is not static inline.
> diff --git a/src/mpstream.h b/src/mpstream.h
> new file mode 100644
> index 0000000..789bd74
> --- /dev/null
> +++ b/src/mpstream.h
> @@ -0,0 +1,124 @@
> +#ifndef TARANTOOL_LUA_MPSTREAM_H_INCLUDED
> +#define TARANTOOL_LUA_MPSTREAM_H_INCLUDED
2. It is not 'LUA' anymore.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH v2 0/2] sql: remove struct Enc
2018-08-27 11:11 [tarantool-patches] [PATCH v2 0/2] sql: remove struct Enc Kirill Shcherbatov
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 1/2] box: export mpstream methods to core Kirill Shcherbatov
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 2/2] sql: remove struct Enc Kirill Shcherbatov
@ 2018-08-28 1:43 ` Vladislav Shpilevoy
2018-08-29 14:12 ` Kirill Yukhin
3 siblings, 0 replies; 10+ messages in thread
From: Vladislav Shpilevoy @ 2018-08-28 1:43 UTC (permalink / raw)
To: tarantool-patches, Kirill Shcherbatov
Hi! Thanks for the patchset!
Next time, please, answer on my review remarks
in a separate reply letter in the old thread if
you send a new version in a new thread.
On 27/08/2018 08:11, Kirill Shcherbatov wrote:
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3545-remove-enc-struct
> Issue: https://github.com/tarantool/tarantool/issues/3545
>
> We shouldn't use legacy Enc structure in SQL as we have
> featured class mpstream in tarantool core. With this path
> all allocations temporally commited with mpstream initialized
Typo: 'commited' -> 'committed'.
> on region and then duplicated in dynamic persistent memory via
> sqlite3DbMallocRaw.
> As a first step, we've moved mpstream API in core lib and reworked
> all legacy interfaces.
>
> Changes in v2:
> - rebased to master, merged with new func
> sql_index_update_table_name.
> - reworked legacy code with new mpstream library API
>
> Kirill Shcherbatov (2):
> box: export mpstream methods to core
> sql: remove struct Enc
>
> src/CMakeLists.txt | 1 +
> src/box/lua/call.c | 11 +-
> src/box/lua/misc.cc | 1 +
> src/box/lua/net_box.c | 127 ++++++------
> src/box/lua/tuple.c | 23 +--
> src/box/sql.c | 473 +++++++++++++++++++++++----------------------
> src/box/sql/build.c | 156 ++++++++-------
> src/box/sql/tarantoolInt.h | 80 +++++---
> src/box/sql/trigger.c | 62 +++---
> src/lua/msgpack.c | 166 ++--------------
> src/lua/msgpack.h | 102 +---------
> src/mpstream.c | 205 ++++++++++++++++++++
> src/mpstream.h | 124 ++++++++++++
> 13 files changed, 845 insertions(+), 686 deletions(-)
> create mode 100644 src/mpstream.c
> create mode 100644 src/mpstream.h
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] sql: remove struct Enc
2018-08-28 1:43 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-08-28 6:46 ` Kirill Shcherbatov
2018-08-28 23:21 ` Vladislav Shpilevoy
0 siblings, 1 reply; 10+ messages in thread
From: Kirill Shcherbatov @ 2018-08-28 6:46 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
> Thanks for the patch! See 7 comments below and a
> commit on the branch.
Hi. Thank you for review
> 1. Double include of region.h.
done.
> 2. Static.
fixed.
> 3. No comment.
/**
* Encode index options @opts into message pack with @stream.
* @param stream Message Pack Stream to use on encode.
* @param opts Index options to encode.
*/
static void
mpstream_encode_index_opts(struct mpstream *stream, struct index_opts *opts)
> 4. Finish a sentence with the dot. Btw the comment
> is not related to the code. Below you do not whether
> the index is unique or not.
> 5. "Did't defined" - corrupted phrase. Rather "did't define".
> But the whole comment looks strange as well as the previous,
> and out of its place. Just remove it.
You know, I've just copied existed peace of code with all comments.
I've get rid off those two comments at all now.
> 6. If 'raw == NULL' you still try to use it in box_update.
My fault. Fixed.
> 7. Here https://github.com/tarantool/tarantool/commit/0d8b0c5641ec4c27547b1d0121bb1e0a641b149b
> I see changes not relevant to this patch and not presented in the
> email. I paste them bellow:
Done.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/2] box: export mpstream methods to core
2018-08-28 1:43 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-08-28 6:46 ` Kirill Shcherbatov
0 siblings, 0 replies; 10+ messages in thread
From: Kirill Shcherbatov @ 2018-08-28 6:46 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
Hi. Thank you for comments.
> 1. As I said in the previous review, reserve and
> reserve_slow are not split in two functions just
> for case. There is a reason. And your patch
> destroys the benefit of reserve/reserve_slow
> splitting. I paste it here:
Sorry for misunderstanding, I didn't catch this words. Only why does mpstream is a header-only library.
-void
-mpstream_flush(struct mpstream *stream);
+static inline void
+mpstream_flush(struct mpstream *stream)
-char *
-mpstream_reserve(struct mpstream *stream, size_t size);
+static inline char *
+mpstream_reserve(struct mpstream *stream, size_t size)
-void
-mpstream_advance(struct mpstream *stream, size_t size);
+static inline void
+mpstream_advance(struct mpstream *stream, size_t size)
> 2. It is not 'LUA' anymore.
Ok, fixed-#ifndef TARANTOOL_LUA_MPSTREAM_H_INCLUDED
-#define TARANTOOL_LUA_MPSTREAM_H_INCLUDED
+#ifndef TARANTOOL_MPSTREAM_H_INCLUDED
+#define TARANTOOL_MPSTREAM_H_INCLUDED
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH v2 2/2] sql: remove struct Enc
2018-08-28 6:46 ` Kirill Shcherbatov
@ 2018-08-28 23:21 ` Vladislav Shpilevoy
0 siblings, 0 replies; 10+ messages in thread
From: Vladislav Shpilevoy @ 2018-08-28 23:21 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches; +Cc: Kirill Yukhin
Thanks for the fixes! Now the whole patchset LGTM.
As I know, for Nikita as well, so Kirill Yu., look at
it, please.
>> 4. Finish a sentence with the dot. Btw the comment
>> is not related to the code. Below you do not whether
>> the index is unique or not.
>> 5. "Did't defined" - corrupted phrase. Rather "did't define".
>> But the whole comment looks strange as well as the previous,
>> and out of its place. Just remove it.
> You know, I've just copied existed peace of code with all comments.
> I've get rid off those two comments at all now.
I know, but it is not an excuse. After the patch was pushed, git blame
would show your name against these lines.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tarantool-patches] Re: [PATCH v2 0/2] sql: remove struct Enc
2018-08-27 11:11 [tarantool-patches] [PATCH v2 0/2] sql: remove struct Enc Kirill Shcherbatov
` (2 preceding siblings ...)
2018-08-28 1:43 ` [tarantool-patches] Re: [PATCH v2 0/2] " Vladislav Shpilevoy
@ 2018-08-29 14:12 ` Kirill Yukhin
3 siblings, 0 replies; 10+ messages in thread
From: Kirill Yukhin @ 2018-08-29 14:12 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov
Hello,
On 27 авг 14:11, Kirill Shcherbatov wrote:
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3545-remove-enc-struct
> Issue: https://github.com/tarantool/tarantool/issues/3545
I've pushed the patchset into 2.0 branch.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-08-29 14:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-27 11:11 [tarantool-patches] [PATCH v2 0/2] sql: remove struct Enc Kirill Shcherbatov
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 1/2] box: export mpstream methods to core Kirill Shcherbatov
2018-08-28 1:43 ` [tarantool-patches] " Vladislav Shpilevoy
2018-08-28 6:46 ` Kirill Shcherbatov
2018-08-27 11:11 ` [tarantool-patches] [PATCH v2 2/2] sql: remove struct Enc Kirill Shcherbatov
2018-08-28 1:43 ` [tarantool-patches] " Vladislav Shpilevoy
2018-08-28 6:46 ` Kirill Shcherbatov
2018-08-28 23:21 ` Vladislav Shpilevoy
2018-08-28 1:43 ` [tarantool-patches] Re: [PATCH v2 0/2] " Vladislav Shpilevoy
2018-08-29 14:12 ` Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox