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’t faced it before actually).
-/*
- * 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;
+ 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 = enc->encode_str(p, "view", 4);
- p = 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 == 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;
+ luamp_encode_str(luaL_msgpack_default, &stream, "checks",
+ strlen("checks"));
+ luamp_encode_array(luaL_msgpack_default, &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);
+ luamp_encode_map(luaL_msgpack_default, &stream, items);
/*
* a[i].pExpr could be NULL for VIEW column names
* represented as checks.
*/
if (a[i].pExpr != NULL) {
- Expr *pExpr = a[i].pExpr;
+ struct 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));
+ 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 != NULL) {
- p = enc->encode_str(p, "name", 4);
- p = 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 = region_used(region) - used;
-/*
- * 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);
+ luamp_encode_map(luaL_msgpack_default, &stream, 2);
/* Mark as unique pk and unique indexes */
- p = 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 = 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);
+ 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 != NULL ? strlen(sql) : 0);
+ mpstream_flush(&stream);
+ if (is_error)
+ return NULL;
+ uint32_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..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 = 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)
+ char *index_opts = NULL, *index_parts = NULL;
+ uint32_t index_opts_sz = 0, index_parts_sz = 0;
+ struct region *region = &fiber()->gc;
+ index_opts =
+ sql_encode_index_opts(region, pIndex, zSql, &index_opts_sz);
+ if (index_opts != NULL) {
+ index_parts =
+ sql_encode_index_parts(region, pIndex, &index_parts_sz);
+ }
+ if (index_opts == NULL || index_parts == NULL) {
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
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
+ }
+ char *raw = sqlite3DbMallocRaw(pParse->db,
+ index_opts_sz + index_parts_sz);
+ if (raw == NULL)
+ return;
+
+ 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,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 == SQL_INDEX_TYPE_NON_UNIQUE ||
@@ -1359,47 +1364,50 @@ 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
+ uint32_t table_stmt_sz = 0, table_opts_stmt_sz = 0;
+ char *table_stmt = NULL, *table_opts_stmt = NULL;
+
+ struct region *region = &fiber()->gc;
+ table_opts_stmt =
+ sql_encode_table_opts(region, table, zStmt,
&table_opts_stmt_sz);
+ if (table_opts_stmt != NULL)
+ table_stmt = sql_encode_table(region, table, &table_stmt_sz);
+ if (table_opts_stmt == NULL || table_stmt == NULL) {
+ pParse->nErr++;
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ return;
}
+ 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);
@@ -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 = fkey_encode_links(fk, FIELD_LINK_PARENT, NULL);
- size_t child_size = fkey_encode_links(fk, FIELD_LINK_CHILD, NULL);
+ struct region *region = &fiber()->gc;
+ uint32_t parent_links_size = 0, child_links_size = 0;
+ char *parent_links = NULL, *child_links = NULL;
+ parent_links = fkey_encode_links(region, fk, FIELD_LINK_PARENT,
+ &parent_links_size);
+ if (parent_links != NULL) {
+ child_links = fkey_encode_links(region, fk, FIELD_LINK_CHILD,
+ &child_links_size);
+ }
+ if (parent_links == NULL || child_links == NULL) {
+ parse_context->nErr++;
+ parse_context->rc = 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 = 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,
+ 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);
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);
-/*
- * Render "format" array for _space entry.
- * Returns result size.
- * If buf==NULL estimate result size.
+/**
+ * Encode Table AST to msgpack on @region.
+ *
+ * @param region to use.
+ * @param table AST to encode.
+ * @param[out] size of result allocation.
+ * @retval NULL on error.
+ * @retval not NULL msgpack pointer 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 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);
/**
- * 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);
-/*
- * Format "parts" array for _index entry.
- * Returns result size.
- * If buf==NULL estimate result size.
+/**
+ * Encode links of given foreign key constraint into MsgPack on
+ * @region.
+ *
+ * @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);
