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 2BB07264AC for ; Tue, 21 Aug 2018 08:09:22 -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 wASbGhCafNpR for ; Tue, 21 Aug 2018 08:09:22 -0400 (EDT) Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 45BBF1FA4F for ; Tue, 21 Aug 2018 08:09:21 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: remove struct Enc From: "n.pettik" In-Reply-To: <792834dabd4287aeb213202581de02b23a4c4557.1534508548.git.kshcherbatov@tarantool.org> Date: Tue, 21 Aug 2018 15:09:17 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <59B7B0ED-9BE2-402D-9953-0347E576BE00@tarantool.org> References: <792834dabd4287aeb213202581de02b23a4c4557.1534508548.git.kshcherbatov@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov In general parch LGTM except several nitpickings. But I hope smb is going to review it as well since I am not sure about proper way of using mpstream (I haven=E2=80=99t faced it before actually). > -/* > - * Format "opts" dictionary for _space entry. > - * Returns result size. > - * If buf=3D=3DNULL 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 =3D get_enc(buf); > - bool is_view =3D pTable !=3D NULL && pTable->def->opts.is_view; > - bool has_checks =3D pTable !=3D NULL && pTable->def->opts.checks = !=3D NULL; > - int checks_cnt =3D has_checks ? pTable->def->opts.checks->nExpr = : 0; > - char *p =3D enc->encode_map(buf, 1 + is_view + (checks_cnt > = 0)); > - > - p =3D enc->encode_str(p, "sql", 3); > - p =3D enc->encode_str(p, zSql, strlen(zSql)); > + assert(sql !=3D NULL); > + size_t used =3D region_used(region); > + struct mpstream stream; > + bool is_error =3D false; > + mpstream_init(&stream, region, region_reserve_cb, = region_alloc_cb, > + set_encode_error, &is_error); > + bool is_view =3D false; > + bool has_checks =3D false; > + if (table !=3D NULL) { > + is_view =3D table->def->opts.is_view; > + has_checks =3D table->def->opts.checks !=3D NULL; > + } > + uint32_t checks_cnt =3D has_checks ? = table->def->opts.checks->nExpr : 0; > + luamp_encode_map(luaL_msgpack_default, &stream, > + 1 + is_view + (checks_cnt > 0)); > + > + luamp_encode_str(luaL_msgpack_default, &stream, "sql", = strlen("sql")); > + luamp_encode_str(luaL_msgpack_default, &stream, sql, = strlen(sql)); > if (is_view) { > - p =3D enc->encode_str(p, "view", 4); > - p =3D enc->encode_bool(p, true); > + luamp_encode_str(luaL_msgpack_default, &stream, "view", > + strlen("view")); > + luamp_encode_bool(luaL_msgpack_default, &stream, true); > } > if (checks_cnt =3D=3D 0) > - return p - buf; > + goto finish; > + > /* Encode checks. */ > - struct ExprList_item *a =3D pTable->def->opts.checks->a; > - p =3D enc->encode_str(p, "checks", 6); > - p =3D enc->encode_array(p, checks_cnt); > - for (int i =3D 0; i < checks_cnt; ++i) { > + struct ExprList_item *a =3D table->def->opts.checks->a; > + luamp_encode_str(luaL_msgpack_default, &stream, "checks", > + strlen("checks")); > + luamp_encode_array(luaL_msgpack_default, &stream, checks_cnt); > + for (uint32_t i =3D 0; i < checks_cnt && !is_error; ++i) { > int items =3D (a[i].pExpr !=3D NULL) + (a[i].zName !=3D = NULL); > - p =3D enc->encode_map(p, items); > + luamp_encode_map(luaL_msgpack_default, &stream, items); > /* > * a[i].pExpr could be NULL for VIEW column names > * represented as checks. > */ I guess this comment is no longer relevant: VIEW column names are stored in def->fields. You can remove additional check: +++ b/src/box/sql.c @@ -1350,17 +1350,11 @@ sql_encode_table_opts(struct region *region, = struct Table *table, for (uint32_t i =3D 0; i < checks_cnt && !is_error; ++i) { int items =3D (a[i].pExpr !=3D NULL) + (a[i].zName !=3D = NULL); mpstream_encode_map(&stream, items); - /* - * a[i].pExpr could be NULL for VIEW column names - * represented as checks. - */ - if (a[i].pExpr !=3D NULL) { - struct Expr *pExpr =3D a[i].pExpr; - assert(pExpr->u.zToken !=3D NULL); - mpstream_encode_str(&stream, "expr", = strlen("expr")); - mpstream_encode_str(&stream, pExpr->u.zToken, + struct Expr *pExpr =3D a[i].pExpr; + assert(pExpr->u.zToken !=3D NULL); + mpstream_encode_str(&stream, "expr", strlen("expr")); + mpstream_encode_str(&stream, pExpr->u.zToken, strlen(pExpr->u.zToken)); - } > if (a[i].pExpr !=3D NULL) { > - Expr *pExpr =3D a[i].pExpr; > + struct Expr *pExpr =3D a[i].pExpr; > assert(pExpr->u.zToken !=3D NULL); > - p =3D enc->encode_str(p, "expr", 4); > - p =3D enc->encode_str(p, pExpr->u.zToken, > - strlen(pExpr->u.zToken)); > + luamp_encode_str(luaL_msgpack_default, &stream, = "expr", > + strlen("expr")); > + luamp_encode_str(luaL_msgpack_default, &stream, > + pExpr->u.zToken, > + strlen(pExpr->u.zToken)); > } > if (a[i].zName !=3D NULL) { > - p =3D enc->encode_str(p, "name", 4); > - p =3D enc->encode_str(p, a[i].zName, = strlen(a[i].zName)); > + luamp_encode_str(luaL_msgpack_default, &stream, = "name", > + strlen("name")); > + luamp_encode_str(luaL_msgpack_default, &stream, > + a[i].zName, = strlen(a[i].zName)); > } > } > - return p - buf; > + > +finish: > + mpstream_flush(&stream); > + if (is_error) > + return NULL; > + uint32_t sz =3D region_used(region) - used; Nitpicking: why do you use uint32_t meanwhile region_used() return = size_t? The same for other functions. > -/* > - * Format "opts" dictionary for _index entry. > - * Returns result size. > - * If buf=3D=3DNULL 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 =3D get_enc(buf); > - char *base =3D buf, *p; > - > - (void)index; > - > - p =3D enc->encode_map(base, 2); > + size_t used =3D region_used(region); > + struct mpstream stream; > + bool is_error =3D false; > + mpstream_init(&stream, region, region_reserve_cb, = region_alloc_cb, > + set_encode_error, &is_error); > + luamp_encode_map(luaL_msgpack_default, &stream, 2); > /* Mark as unique pk and unique indexes */ > - p =3D enc->encode_str(p, "unique", 6); > + luamp_encode_str(luaL_msgpack_default, &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 +1529,20 @@ int tarantoolSqlite3MakeIdxOpts(SqliteIndex = *index, const char *zSql, void *buf) > * INSERT OR REPLACE/IGNORE uniqueness checks will be also done = by > * Tarantool. > */ > - p =3D enc->encode_bool(p, IsUniqueIndex(index)); > - p =3D enc->encode_str(p, "sql", 3); > - p =3D enc->encode_str(p, zSql, zSql ? strlen(zSql) : 0); > - return (int)(p - base); > + luamp_encode_bool(luaL_msgpack_default, &stream, = IsUniqueIndex(index)); > + luamp_encode_str(luaL_msgpack_default, &stream, "sql", = strlen("sql")); > + luamp_encode_str(luaL_msgpack_default, &stream, sql, > + sql !=3D NULL ? strlen(sql) : 0); > + mpstream_flush(&stream); > + if (is_error) > + return NULL; > + uint32_t sz =3D region_used(region) - used; > + char *raw =3D region_join(region, sz); > + if (raw =3D=3D NULL) > + diag_set(OutOfMemory, sz, "region_join", "raw"); > + else > + *size =3D sz; > + return raw; > } >=20 > void > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index dddeb12..ca4ba44 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -1241,25 +1241,30 @@ createIndex(Parse * pParse, Index * pIndex, = int iSpaceId, int iIndexId, > Vdbe *v =3D sqlite3GetVdbe(pParse); > int iFirstCol =3D ++pParse->nMem; > int iRecord =3D (pParse->nMem +=3D 6); /* 6 total columns */ > - char *zOpts, *zParts; > - int zOptsSz, zPartsSz; > - > - /* Format "opts" and "parts" for _index entry. */ > - zOpts =3D sqlite3DbMallocRaw(pParse->db, > - tarantoolSqlite3MakeIdxOpts(pIndex, = zSql, > - NULL) + > - tarantoolSqlite3MakeIdxParts(pIndex, > - NULL) + = 2); > - if (!zOpts) > + char *index_opts =3D NULL, *index_parts =3D NULL; Nitpicking: I would combine declaration and first usage. The same for index_opts_sz and index_parts_sz. Overall, consider following refactoring: +++ b/src/box/sql/build.c @@ -1241,22 +1241,19 @@ createIndex(Parse * pParse, Index * pIndex, int = iSpaceId, int iIndexId, Vdbe *v =3D sqlite3GetVdbe(pParse); int iFirstCol =3D ++pParse->nMem; int iRecord =3D (pParse->nMem +=3D 6); /* 6 total columns = */ - char *index_opts =3D NULL, *index_parts =3D NULL; - uint32_t index_opts_sz =3D 0, index_parts_sz =3D 0; struct region *region =3D &fiber()->gc; - index_opts =3D - sql_encode_index_opts(region, pIndex, zSql, = &index_opts_sz); - if (index_opts !=3D NULL) { - index_parts =3D - sql_encode_index_parts(region, pIndex, = &index_parts_sz); - } - if (index_opts =3D=3D NULL || index_parts =3D=3D NULL) { - pParse->rc =3D SQL_TARANTOOL_ERROR; - pParse->nErr++; - return; - } + uint32_t index_opts_sz =3D 0; + char *index_opts =3D sql_encode_index_opts(region, pIndex, zSql, + &index_opts_sz); + if (index_opts =3D=3D NULL) + goto error; + uint32_t index_parts_sz =3D 0; + char *index_parts =3D sql_encode_index_parts(region, pIndex, + &index_parts_sz); + if (index_parts =3D=3D NULL) + goto error; char *raw =3D sqlite3DbMallocRaw(pParse->db, - index_opts_sz + index_parts_sz); + index_opts_sz + index_parts_sz); if (raw =3D=3D NULL) return; =20 @@ -1310,6 +1307,10 @@ createIndex(Parse * pParse, Index * pIndex, int = iSpaceId, int iIndexId, if (pIndex->index_type =3D=3D SQL_INDEX_TYPE_NON_UNIQUE || pIndex->index_type =3D=3D SQL_INDEX_TYPE_UNIQUE) sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE); + return; +error: + pParse->rc =3D SQL_TARANTOOL_ERROR; + pParse->nErr++; } > + uint32_t index_opts_sz =3D 0, index_parts_sz =3D 0; > + struct region *region =3D &fiber()->gc; > + index_opts =3D > + sql_encode_index_opts(region, pIndex, zSql, = &index_opts_sz); Why do you pass region as param if you anyway get it from global = fiber()? > + if (index_opts !=3D NULL) { > + index_parts =3D > + sql_encode_index_parts(region, pIndex, = &index_parts_sz); > + } > + if (index_opts =3D=3D NULL || index_parts =3D=3D NULL) { > + pParse->rc =3D SQL_TARANTOOL_ERROR; > + pParse->nErr++; > return; > - zOptsSz =3D tarantoolSqlite3MakeIdxOpts(pIndex, zSql, zOpts); > - zParts =3D zOpts + zOptsSz + 1; > - zPartsSz =3D tarantoolSqlite3MakeIdxParts(pIndex, zParts); > -#if SQLITE_DEBUG > - /* NUL-termination is necessary for VDBE trace facility only */ > - zOpts[zOptsSz] =3D 0; > - zParts[zPartsSz] =3D 0; > -#endif > + } > + char *raw =3D sqlite3DbMallocRaw(pParse->db, > + index_opts_sz + index_parts_sz); Wrong indentation. > + if (raw =3D=3D NULL) > + return; > + > + memcpy(raw, index_opts, index_opts_sz); > + index_opts =3D raw; > + raw +=3D index_opts_sz; > + memcpy(raw, index_parts, index_parts_sz); > + index_parts =3D raw; >=20 > if (pParse->pNewTable) { > int reg; > @@ -1295,11 +1300,11 @@ 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 =3D=3D SQL_INDEX_TYPE_NON_UNIQUE || > @@ -1359,47 +1364,50 @@ makeIndexSchemaRecord(Parse * pParse, > static void > createSpace(Parse * pParse, int iSpaceId, char *zStmt) The same comments to this functions as to previous one and the same refactoring. > { > - Table *p =3D pParse->pNewTable; > + struct Table *table =3D pParse->pNewTable; > Vdbe *v =3D sqlite3GetVdbe(pParse); > int iFirstCol =3D ++pParse->nMem; > int iRecord =3D (pParse->nMem +=3D 7); > - char *zOpts, *zFormat; > - int zOptsSz, zFormatSz; > - > - zOpts =3D sqlite3DbMallocRaw(pParse->db, > - tarantoolSqlite3MakeTableFormat(p, = NULL) + > - tarantoolSqlite3MakeTableOpts(p, = zStmt, > - NULL) + = 2); > - if (!zOpts) { > - zOptsSz =3D 0; > - zFormat =3D NULL; > - zFormatSz =3D 0; > - } else { > - zOptsSz =3D tarantoolSqlite3MakeTableOpts(p, zStmt, = zOpts); > - zFormat =3D zOpts + zOptsSz + 1; > - zFormatSz =3D tarantoolSqlite3MakeTableFormat(p, = zFormat); > -#if SQLITE_DEBUG > - /* NUL-termination is necessary for VDBE-tracing = facility only */ > - zOpts[zOptsSz] =3D 0; > - zFormat[zFormatSz] =3D 0; > -#endif > + uint32_t table_stmt_sz =3D 0, table_opts_stmt_sz =3D 0; > + char *table_stmt =3D NULL, *table_opts_stmt =3D NULL; > + > + struct region *region =3D &fiber()->gc; > + table_opts_stmt =3D > + sql_encode_table_opts(region, table, zStmt, = &table_opts_stmt_sz); Out of 80 chars. > + if (table_opts_stmt !=3D NULL) > + table_stmt =3D sql_encode_table(region, table, = &table_stmt_sz); > + if (table_opts_stmt =3D=3D NULL || table_stmt =3D=3D NULL) { > + pParse->nErr++; > + pParse->rc =3D SQL_TARANTOOL_ERROR; > + return; > } > + char *raw =3D sqlite3DbMallocRaw(pParse->db, > + table_stmt_sz + = table_opts_stmt_sz); > + if (raw =3D=3D NULL) > + return; > + > + memcpy(raw, table_opts_stmt, table_opts_stmt_sz); > + table_opts_stmt =3D raw; > + raw +=3D table_opts_stmt_sz; > + memcpy(raw, table_stmt, table_stmt_sz); > + table_stmt =3D raw; >=20 > 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); > @@ -1603,26 +1611,42 @@ 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 =3D fkey_encode_links(fk, FIELD_LINK_PARENT, = NULL); > - size_t child_size =3D fkey_encode_links(fk, FIELD_LINK_CHILD, = NULL); > + struct region *region =3D &fiber()->gc; > + uint32_t parent_links_size =3D 0, child_links_size =3D 0; > + char *parent_links =3D NULL, *child_links =3D NULL; > + parent_links =3D fkey_encode_links(region, fk, = FIELD_LINK_PARENT, > + &parent_links_size); Consider refactoring: @@ -1612,20 +1613,16 @@ vdbe_emit_fkey_create(struct Parse = *parse_context, const struct fkey_def *fk) sqlite3VdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 6, 0, fkey_action_strs[fk->on_update], P4_STATIC); struct region *region =3D &fiber()->gc; - uint32_t parent_links_size =3D 0, child_links_size =3D 0; - char *parent_links =3D NULL, *child_links =3D NULL; - parent_links =3D fkey_encode_links(region, fk, = FIELD_LINK_PARENT, - &parent_links_size); - if (parent_links !=3D NULL) { - child_links =3D fkey_encode_links(region, fk, = FIELD_LINK_CHILD, - &child_links_size); - } - if (parent_links =3D=3D NULL || child_links =3D=3D NULL) { - parse_context->nErr++; - parse_context->rc =3D SQL_TARANTOOL_ERROR; - sqlite3DbFree(parse_context->db, name_copy); - return; - } + uint32_t parent_links_size; + char *parent_links =3D fkey_encode_links(region, fk, = FIELD_LINK_PARENT, + &parent_links_size); + if (parent_links =3D=3D NULL) + goto tnt_error; + uint32_t child_links_size; + char *child_links =3D fkey_encode_links(region, fk, = FIELD_LINK_CHILD, + &child_links_size); + if (child_links =3D=3D NULL) + goto tnt_error; /* * We are allocating memory for both parent and child * arrays in the same chunk. Thus, first OP_Blob opcode @@ -1633,7 +1630,7 @@ vdbe_emit_fkey_create(struct Parse *parse_context, = const struct fkey_def *fk) * as dynamic and releases memory. */ char *raw =3D sqlite3DbMallocRaw(parse_context->db, - parent_links_size + = child_links_size); + parent_links_size + = child_links_size); if (raw =3D=3D NULL) { sqlite3DbFree(parse_context->db, name_copy); return; @@ -1654,6 +1651,10 @@ vdbe_emit_fkey_create(struct Parse = *parse_context, const struct fkey_def *fk) constr_tuple_reg + 9); sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE); sqlite3ReleaseTempRange(parse_context, constr_tuple_reg, 10); +tnt_error: + parse_context->nErr++; + parse_context->rc =3D SQL_TARANTOOL_ERROR; + sqlite3DbFree(parse_context->db, name_copy); > + if (parent_links !=3D NULL) { > + child_links =3D fkey_encode_links(region, fk, = FIELD_LINK_CHILD, > + &child_links_size); > + } > + if (parent_links =3D=3D NULL || child_links =3D=3D NULL) { > + parse_context->nErr++; > + parse_context->rc =3D SQL_TARANTOOL_ERROR; > + sqlite3DbFree(parse_context->db, name_copy); > + return; > + } > /* > * 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 =3D sqlite3DbMallocRaw(parse_context->db, > - parent_size + = child_size); > - if (parent_links =3D=3D NULL) { > - sqlite3DbFree(parse_context->db, (void *) name_copy); > + char *raw =3D sqlite3DbMallocRaw(parse_context->db, > + parent_links_size + = child_links_size); > + if (raw =3D=3D NULL) { > + sqlite3DbFree(parse_context->db, name_copy); > return; > } > - parent_size =3D fkey_encode_links(fk, FIELD_LINK_PARENT, = parent_links); > - char *child_links =3D parent_links + parent_size; > - child_size =3D 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 =3D raw; > + raw +=3D parent_links_size; > + memcpy(raw, child_links, child_links_size); > + child_links =3D 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, > + sqlite3VdbeAddOp4(vdbe, OP_Blob, parent_links_size, = constr_tuple_reg + 8, Out of 80 chars. > SQL_SUBTYPE_MSGPACK, parent_links, = P4_DYNAMIC); > sqlite3VdbeAddOp3(vdbe, OP_MakeRecord, constr_tuple_reg, 9, > constr_tuple_reg + 9); > diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h > index 94517f6..b158cc7 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); >=20 > -/* > - * Render "format" array for _space entry. > - * Returns result size. > - * If buf=3D=3DNULL estimate result size. > +/** > + * Encode Table AST to msgpack on @region. Table doesn=E2=80=99t feature any AST. I like previous comment: =E2=80=9CEncode format as entry to be inserted to _space=E2=80=9D or = smth like that. > + * > + * @param region to use. Don=E2=80=99t cut arg names. Ex.: @param region Region to use The same for other functions. > + * @param table AST to encode. > + * @param[out] size of result allocation. > + * @retval NULL on error. > + * @retval not NULL msgpack pointer on success. Pointer to encoded into mspack format? > */ > -int tarantoolSqlite3MakeTableFormat(Table * pTable, void *buf); > +char * > +sql_encode_table(struct region *region, struct Table *table, uint32_t = *size); >=20 > -/* > - * Format "opts" dictionary for _space entry. > - * Returns result size. > - * If buf=3D=3DNULL estimate result size. > +/** > + * Encode "opts" dictionary for _space entry on @region. > + * > + * @param region to use. > + * @param table AST containing opts to encode. > + * @param sql source request to encode. > + * @param[out] size of result allocation. > + * @retval NULL on error. > + * @retval not NULL msgpack pointer 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); >=20 > /** > - * Encode links of given foreign key constraint into MsgPack. > + * Encode links of given foreign key constraint into MsgPack on > + * @region. > * > + * @param 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 of result allocation. > + * @retval NULL on error. > + * @retval not NULL msgpack pointer 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); >=20 > -/* > - * Format "parts" array for _index entry. > - * Returns result size. > - * If buf=3D=3DNULL estimate result size. > +/** > + * Encode links of given foreign key constraint into MsgPack on > + * @region. ??? I guess comment doesn=E2=80=99t belong to this function. > + * > + * @param region to use. > + * @param index to encode. > + * @param[out] size of result allocation. > + * @retval NULL on error. > + * @retval not NULL msgpack pointer on success. > */ > -int tarantoolSqlite3MakeIdxParts(Index * index, void *buf); > +char * > +sql_encode_index_parts(struct region *region, struct Index *index, > + uint32_t *size); >=20