From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 66B9A2960B for ; Tue, 21 Aug 2018 11:50:31 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Iw5DAFnjbQJu for ; Tue, 21 Aug 2018 11:50:31 -0400 (EDT) Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id CF8EE295BB for ; Tue, 21 Aug 2018 11:50:30 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: remove struct Enc References: <792834dabd4287aeb213202581de02b23a4c4557.1534508548.git.kshcherbatov@tarantool.org> <59B7B0ED-9BE2-402D-9953-0347E576BE00@tarantool.org> From: Kirill Shcherbatov Message-ID: <6bdaa379-f063-0364-49ca-a5ad573c6975@tarantool.org> Date: Tue, 21 Aug 2018 18:50:27 +0300 MIME-Version: 1.0 In-Reply-To: <59B7B0ED-9BE2-402D-9953-0347E576BE00@tarantool.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: "n.pettik" , tarantool-patches@freelists.org > In general parch LGTM except several nitpickings. Hi! Thank you for review. > I guess this comment is no longer relevant: VIEW column names are > stored in def->fields. You can remove additional check: - /* - * a[i].pExpr could be NULL for VIEW column names - * represented as checks. - */ - if (a[i].pExpr != NULL) { - struct Expr *pExpr = a[i].pExpr; - assert(pExpr->u.zToken != NULL); - mpstream_encode_str(&stream, "expr", strlen("expr")); - mpstream_encode_str(&stream, pExpr->u.zToken, - strlen(pExpr->u.zToken)); - } + assert(a[i].pExpr != NULL); + struct Expr *pExpr = a[i].pExpr; + assert(pExpr->u.zToken != NULL); + mpstream_encode_str(&stream, "expr", strlen("expr")); + mpstream_encode_str(&stream, pExpr->u.zToken, + strlen(pExpr->u.zToken)); > Nitpicking: why do you use uint32_t meanwhile region_used() return size_t? > The same for other functions. done. > Nitpicking: I would combine declaration and first usage. > The same for index_opts_sz and index_parts_sz. > Overall, consider following refactoring: Done. > Why do you pass region as param if you anyway get it from global fiber()? - struct region *region = &fiber()->gc; + struct region *region = &pParse->region; > Wrong indentation. Fixed. > The same comments to this functions as to previous one > and the same refactoring. done > Out of 80 chars. done > Pointer to encoded into mspack format? > ??? I guess comment doesn’t belong to this function. Fixed. =========================================== diff --git a/src/box/sql.c b/src/box/sql.c index ae12cae..28ad9d3 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; @@ -1190,66 +1192,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. */ @@ -1278,34 +1220,38 @@ 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) +static void +set_encode_error(void *error_ctx) { - 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; + bool *is_error = error_ctx; + *is_error = true; +} - p = enc->encode_array(base, n); +char * +sql_encode_table(struct region *region, struct Table *table, uint32_t *size) +{ + 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]; @@ -1315,10 +1261,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", strlen("name")); + mpstream_encode_str(&stream, field->name, strlen(field->name)); + mpstream_encode_str(&stream, "type", strlen("type")); if (i == pk_forced_int) { t = "integer"; } else { @@ -1329,108 +1275,149 @@ int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf) } 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, strlen(t)); + mpstream_encode_str(&stream, "affinity", strlen("affinity")); + mpstream_encode_uint(&stream, + def->fields[i].affinity); + mpstream_encode_str(&stream, "is_nullable", + strlen("is_nullable")); + mpstream_encode_bool(&stream, + def->fields[i].is_nullable); + mpstream_encode_str(&stream, "nullable_action", + strlen("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, strlen(action)); if (cid != COLL_NONE) { - p = enc->encode_str(p, "collation", strlen("collation")); - p = enc->encode_uint(p, cid); + mpstream_encode_str(&stream, "collation", + strlen("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", + strlen("default")); + mpstream_encode_str(&stream, default_str, + strlen(default_str)); } } - return (int)(p - base); + mpstream_flush(&stream); + if (is_error) + return NULL; + size_t sz = region_used(region) - used; + char *raw = region_join(region, sz); + if (raw == NULL) + diag_set(OutOfMemory, sz, "region_join", "raw"); + else + *size = sz; + 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; + bool has_checks = false; + if (table != NULL) { + is_view = table->def->opts.is_view; + has_checks = table->def->opts.checks != NULL; + } + uint32_t checks_cnt = has_checks ? table->def->opts.checks->nExpr : 0; + mpstream_encode_map(&stream, 1 + is_view + (checks_cnt > 0)); + + mpstream_encode_str(&stream, "sql", strlen("sql")); + mpstream_encode_str(&stream, sql, strlen(sql)); if (is_view) { - p = enc->encode_str(p, "view", 4); - p = enc->encode_bool(p, true); + mpstream_encode_str(&stream, "view", strlen("view")); + mpstream_encode_bool(&stream, true); } if (checks_cnt == 0) - return p - buf; + goto finish; + /* 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) { + struct ExprList_item *a = table->def->opts.checks->a; + mpstream_encode_str(&stream, "checks", strlen("checks")); + mpstream_encode_array(&stream, checks_cnt); + for (uint32_t i = 0; i < checks_cnt && !is_error; ++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)); - } + mpstream_encode_map(&stream, items); + assert(a[i].pExpr != NULL); + struct Expr *pExpr = a[i].pExpr; + assert(pExpr->u.zToken != NULL); + mpstream_encode_str(&stream, "expr", strlen("expr")); + mpstream_encode_str(&stream, 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)); + mpstream_encode_str(&stream, "name", strlen("name")); + mpstream_encode_str(&stream, a[i].zName, + strlen(a[i].zName)); } } - return p - buf; + +finish: + mpstream_flush(&stream); + if (is_error) + return NULL; + size_t sz = region_used(region) - used; + char *raw = region_join(region, sz); + if (raw == NULL) + diag_set(OutOfMemory, sz, "region_join", "raw"); + else + *size = sz; + 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) + return NULL; + size_t sz = region_used(region) - used; + char *raw = region_join(region, sz); + if (raw == NULL) + diag_set(OutOfMemory, sz, "region_join", "raw"); + else + *size = sz; + 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. */ + sqlite3PrimaryKeyIndex(index->pTable); + struct field_def *fields = index->pTable->def->fields; 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) @@ -1443,9 +1430,11 @@ 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; + uint32_t part_count = key_def->part_count; struct key_part *part = key_def->parts; - char *p = enc->encode_array(base, key_def->part_count); - for (uint32_t i = 0; i < key_def->part_count; ++i, ++part) { + mpstream_encode_array(&stream, part_count); + for (uint32_t i = 0; i < part_count; ++i, ++part) { uint32_t col = part->fieldno; assert(fields[col].is_nullable == action_is_nullable(fields[col].nullable_action)); @@ -1456,54 +1445,60 @@ 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", strlen("type")); + mpstream_encode_str(&stream, t, strlen(t)); + mpstream_encode_str(&stream, "field", strlen("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", + strlen("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", + strlen("is_nullable")); + mpstream_encode_bool(&stream, + fields[col].is_nullable); + mpstream_encode_str(&stream, "nullable_action", + strlen("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, strlen(action_str)); - p = enc->encode_str(p, "sort_order", 10); + mpstream_encode_str(&stream, "sort_order", + strlen("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, + strlen(sort_order_str)); } - return p - base; + mpstream_flush(&stream); + if (is_error) + return NULL; + size_t sz = region_used(region) - used; + char *raw = region_join(region, sz); + if (raw == NULL) + diag_set(OutOfMemory, sz, "region_join", "raw"); + else + *size = sz; + return raw; } -/* - * Format "opts" dictionary for _index entry. - * Returns result size. - * If buf==NULL estimate result size. - * - * Ex: { - * "unique": "true", - * "sql": "CREATE INDEX student_by_name ON students(name)" - * } - */ -int tarantoolSqlite3MakeIdxOpts(SqliteIndex *index, const char *zSql, void *buf) +char * +sql_encode_index_opts(struct region *region, struct SqliteIndex *index, + const char *sql, uint32_t *size) { - const struct Enc *enc = get_enc(buf); - char *base = buf, *p; - - (void)index; - - p = enc->encode_map(base, 2); + 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_map(&stream, 1 + (sql != NULL)); /* Mark as unique pk and unique indexes */ - p = enc->encode_str(p, "unique", 6); + mpstream_encode_str(&stream, "unique", strlen("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 @@ -1511,10 +1506,21 @@ int tarantoolSqlite3MakeIdxOpts(SqliteIndex *index, const char *zSql, void *buf) * INSERT OR REPLACE/IGNORE uniqueness checks will be also done by * Tarantool. */ - p = enc->encode_bool(p, IsUniqueIndex(index)); - p = enc->encode_str(p, "sql", 3); - p = enc->encode_str(p, zSql, zSql ? strlen(zSql) : 0); - return (int)(p - base); + mpstream_encode_bool(&stream, IsUniqueIndex(index)); + if (sql != NULL) { + mpstream_encode_str(&stream, "sql", strlen("sql")); + mpstream_encode_str(&stream, sql, strlen(sql)); + } + mpstream_flush(&stream); + if (is_error) + return NULL; + size_t sz = region_used(region) - used; + char *raw = region_join(region, sz); + if (raw == NULL) + diag_set(OutOfMemory, sz, "region_join", "raw"); + else + *size = sz; + return raw; } void diff --git a/src/box/sql/build.c b/src/box/sql/build.c index dddeb12..462b6e6 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1241,25 +1241,27 @@ 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. */ - zOpts = sqlite3DbMallocRaw(pParse->db, - tarantoolSqlite3MakeIdxOpts(pIndex, zSql, - 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, pIndex, zSql, &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 = tarantoolSqlite3MakeIdxOpts(pIndex, zSql, 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; @@ -1295,16 +1297,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++; } /* @@ -1359,50 +1365,55 @@ 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; } /* @@ -1603,33 +1614,53 @@ 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) { + sqlite3DbFree(parse_context->db, name_copy); 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; + sqlite3DbFree(parse_context->db, name_copy); } /** diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h index 94517f6..6a42ed3 100644 --- a/src/box/sql/tarantoolInt.h +++ b/src/box/sql/tarantoolInt.h @@ -137,46 +137,74 @@ 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 on 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 on 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 region 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 on 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 on 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. - * Returns result size. - * If buf==NULL estimate result size. +/** + * Encode "opts" dictionary for _index entry on @region. + * + * @param region region to use. + * @param index index to encode. + * @param sql source request to encode. + * @param[out] size size of result allocation. + * @retval NULL on error. + * @retval not NULL pointer to msgpack on success */ -int tarantoolSqlite3MakeIdxOpts(Index * index, const char *zSql, void *buf); +char * +sql_encode_index_opts(struct region *region, struct Index *index, + const char *sql, 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..6a07081 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -225,20 +225,22 @@ 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) + uint32_t opts_buff_sz = 0; + char *data = sql_encode_table_opts(&fiber()->gc, NULL, sql_str, + &opts_buff_sz); + if (data != NULL) { + opts_buff = sqlite3DbMallocRaw(parse->db, opts_buff_sz); + if (opts_buff != NULL) + memcpy(opts_buff, data, opts_buff_sz); + } else { + parse->nErr++; + parse->rc = SQL_TARANTOOL_ERROR; + } + if (opts_buff == NULL) goto triggerfinish_cleanup; - int opts_buff_sz = - tarantoolSqlite3MakeTableOpts(0, sql_str, opts_buff); - trigger_name = sqlite3DbStrDup(parse->db, trigger_name); - if (db->mallocFailed) + if (trigger_name == NULL) goto triggerfinish_cleanup; sqlite3VdbeAddOp4(v,