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 3DDAC280ED for ; Fri, 23 Mar 2018 12:20:52 -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 koBoV48lKy0Z for ; Fri, 23 Mar 2018 12:20:52 -0400 (EDT) Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (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 469AD27FC5 for ; Fri, 23 Mar 2018 12:20:51 -0400 (EDT) From: "n.pettik" Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_1CC5F354-A9F6-48FE-B883-6A3353129CF4" Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH 2/4] sql: introduce opcodes to operate on system spaces Date: Fri, 23 Mar 2018 19:20:48 +0300 In-Reply-To: References: <6AAB3A70-E619-4232-AFC9-5EAB27D03961@tarantool.org> <430BF95D-84E2-4A63-915D-A079C5E97285@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: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org --Apple-Mail=_1CC5F354-A9F6-48FE-B883-6A3353129CF4 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 22 Mar 2018, at 16:09, v.shpilevoy@tarantool.org wrote: >=20 >=20 >=20 >> 22 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2018 =D0=B3., =D0=B2 15:23, = n.pettik > = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >>=20 >>=20 >>> On 22 Mar 2018, at 14:42, v.shpilevoy@tarantool.org = wrote: >>>=20 >>> See below 6 comments. >>>=20 >>>> 21 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2018 =D0=B3., =D0=B2 2:48, Nikita = Pettik > = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >>>>=20 >>>> Since it is impossible to use space pointers during execution of = DDL >>>> (after any DDL schema may change and pointers fetched at compile = time >>>> can become expired), special opcodes to operate on system spaces = have >>>> been introduced: OP_SInsert and OP_SDelete. They take space id as >>>> an argument and make space lookup each time before insertion or = deletion. >>>> However, sometimes it is still required to iterate through space = (e.g. >>>> to satisfy WHERE clause) during DDL. As far as now cursors rely on >>>> pointers to space, it is required to convert space id to space = pointer >>>> during VDBE execution. Hence, another opcode is added: OP_SIDtoPtr. >>>> Finally, existing opcodes, which are invoked only during DDL, have = been >>>> also refactored. >>>>=20 >>>> Part of #3252 >>>> --- >>>> src/box/sql.c | 51 ++++++++++++------- >>>> src/box/sql/opcodes.c | 45 +++++++++-------- >>>> src/box/sql/opcodes.h | 53 ++++++++++---------- >>>> src/box/sql/tarantoolInt.h | 11 ++--- >>>> src/box/sql/vdbe.c | 121 = ++++++++++++++++++++++++++++++++++----------- >>>> 5 files changed, 182 insertions(+), 99 deletions(-) >>>>=20 >>>> diff --git a/src/box/sql.c b/src/box/sql.c >>>> index e943131ae..db4c7e7ea 100644 >>>> --- a/src/box/sql.c >>>> +++ b/src/box/sql.c >>>> +int >>>> +sql_delete_by_key(struct space *space, char *key, uint32_t = key_size) >>>> +{ >>>> struct request request; >>>> memset(&request, 0, sizeof(request)); >>>> request.type =3D IPROTO_DELETE; >>>> request.key =3D key; >>>> request.key_end =3D key + key_size; >>>> - request.space_id =3D pCur->space->def->id; >>>> - rc =3D sql_execute_dml(&request, pCur->space); >>>> + request.space_id =3D space->def->id; >>>> + int rc =3D sql_execute_dml(&request, space); >>>=20 >>> 1. Why do you need sql_execute_dml? It does exactly the same as = process_rw, but process_rw can also return a tuple. And this process_rw = feature is needed for the patch on last inserted tuple returning. >>=20 >> I need it as an analogue of process_rw(), since process_rw() is = unavailable=20 >> via public BOX interface (declared as static). If you approve to make = it public, >> where should I place it and should I rename it? >> Or, for instance, make sql_execute_dml be complete copy of = process_rw()? >> It is unlikely to be useful somewhere except for SQL (and box = internals surely). >> Initial indent was to reduce overhead while calling box functions = from SQL bindings (sql.c): >> ordinary call stack looks like: box_insert() -> box_process1() -> = space lookup + process_rw(). >> In such scheme we also process redundant space lookup since we = already have struct space * in cursor. >> (Patch where I introduced this function isn=E2=80=99t in trunk, so I = can easily fix it). >=20 > Yes, I very like the removal of box_process1. I just do not like code = duplicate. But also I can not see a place for declaration of process_rw = in any header. So lets make it extern "C" non-static in box.cc = , and declare it as extern in sql.c. Ok, I=E2=80=99m attaching patch to email (since it belongs to another = patch-set: np/gh-3122-remove-pgnoRoot) >=20 >>=20 >>>>=20 >>>> @@ -1088,20 +1107,16 @@ int tarantoolSqlite3IncrementMaxid(BtCursor = *pCur) >>>> request.ops =3D ops; >>>> request.ops_end =3D ops + sizeof(ops); >>>> request.type =3D IPROTO_UPSERT; >>>> - request.space_id =3D pCur->space->def->id; >>>> - rc =3D sql_execute_dml(&request, pCur->space); >>>> + request.space_id =3D BOX_SCHEMA_ID; >>>> + rc =3D sql_execute_dml(&request, space_schema); >>>=20 >>> 2. If you use process_rw, then you do not need iterator to get the = previous max_id and increment it - you just do process_rw, get result = and do tuple_field_u64(result, 1, space_max_id); >=20 > Suppose, this will be fixed, if you will use process_rw, as described = above. Fixed (I am attaching changes at the end of mail). >=20 >>>=20 >>>> if (rc !=3D 0) { >>>> iterator_delete(it); >>>> return SQL_TARANTOOL_ERROR; >>>> } >>>> - if (pCur->last_tuple !=3D NULL) { >>>> - box_tuple_unref(pCur->last_tuple); >>>> - } >>>> - box_tuple_ref(res); >>>> - pCur->last_tuple =3D res; >>>> - pCur->eState =3D CURSOR_VALID; >>>> + rc =3D tuple_field_u64(res, 1, space_max_id); >>>> + (*space_max_id)++; >>>> iterator_delete(it); >>>> - return SQLITE_OK; >>>> + return rc =3D=3D 0 ? SQLITE_OK : SQL_TARANTOOL_ERROR; >>>> } >>>>=20 >>>> /* >>>> @@ -1687,14 +1702,12 @@ int = tarantoolSqlite3EphemeralGetMaxId(BtCursor *pCur, uint32_t fieldno, >>>> * If index is empty - return 0 in max_id and success status >>>> */ >>>> int >>>> -tarantoolSqlGetMaxId(BtCursor *cur, uint32_t fieldno, >>>> - uint64_t *max_id) >>>> +tarantoolSqlGetMaxId(uint32_t space_id, uint64_t *max_id) >>>=20 >>> 3. After changing a function signature, please, fix a comments too. = And why this function is sure, that the max id stored in the first = column? And why it can not be used for secondary indexes? >>=20 >> It might, but it is only applied for _sequence space during DDL = routine >> (and this is also answer for the first question: format of system = space doesn=E2=80=99t change frequently), >> so I decided to remove cursor/index overhead (as a part of overall = DDL refactoring). >> Probably, I should also rename this opcode to make it more clear. >=20 > Yes, lets rename it, and remove space_id argument, if it is always = used for _sequence space. Done (I am attaching changes at the end of mail). >>>>=20 >>>> +/* Opcode: SInsert P1 P2 * * P5 >>>> + * Synopsis: space id =3D P1, key =3D r[P2] >>>> + * >>>> + * This opcode is used only during DML routine. >>>=20 >>> 4. Do you mean DDL instead of DML? >=20 > Hm? Fixed (at both places). >=20 >>>=20 >>>> + * In contrast to ordinary insertion, insertion to system spaces >>>> + * such as _space or _index will lead to schema changes. >>>> + * Thus, usage of space pointers is going to be impossible, >>>> + * as far as pointers can be expired since compilation time. >>>> + * >>>> + * If P5 is set to OPFLAG_NCHANGE, account overall changes >>>> + * made to database. >>>> + */ >>>> +case OP_SInsert: { >>>> + assert(pOp->p1 > 0); >>>> + >>>> +/* Opcode: SIDtoPtr P1 P2 * * * >>>> + * Synopsis: space id =3D P1, space[out] =3D r[P2] >>>> + * >>>> + * This opcode makes look up by space id and save found space >>>> + * into register, specified by the content of register P2. >>>> + * Such trick is needed during DML routine, since schema may >>>> + * change and pointers become expired. >>>> + */ >>>> +case OP_SIDtoPtr: { >>>=20 >>> 6. It seems to be unused in the patch. Can you use it for all these = things, introduced by the previous patch? >>> struct space *space =3D = space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum)); >>> assert(space !=3D NULL); >>> sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void *) = space); >>> sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, pIdx->tnum, = space_ptr_reg); >>>=20 >>> Using OP_SIDtoPtr you already in this patch can defer space_by_id = lookup. >>=20 >> It is used in the next patch. I don=E2=80=99t want to make a lot of = diffs, since introducing of any opcodes >> leads to auto-generating of opcodes.c/h and as a result to vast = amount of redundant diff. >=20 > Ok, then lets leave it in this patch. >=20 >> As for your example, during SQL DDL we need to execute queries such = as: >> 'DELETE FROM BOX_SEQUENCE WHERE name =3D zName;=E2=80=99 >> And =E2=80=98name=E2=80=99 is not a key. Hence, we have to open = cursor and iterate through the space. >> But if we used pointers to spaces which were fetched at compile = state, >> they would become expired at VDBE runtime: >> after first stage of DDL (e.g. deletion from _trigger), schema may = change. >> Moreover, we are going to support syntax like: =E2=80=98CREATE TABLE = AS SELECT =E2=80=A6=E2=80=99, >> where DDL and DML are mixing. Thus we need facility to make space = lookup at VDBE runtime. >=20 > I agree, that we must have this opcode, but why it can not be used for = DML too? In my example you at compile time get space *, store it by = OP_LoadPtr, and then use to open iterator (but I can not see where - I = commented this in a previous patch review), but for DML you can replace = compile lookups on runtime lookups using OP_SIDtoPtr. And it also solves = the problem, that a prepared VDBE statement (which in a future we will = store for a long time) must store space * in opcodes - we can use = OP_SIDtoPtr and do not store it until execution. Sorry, can not explain = more clear. We can also discuss it face-to-face. Now I get what you mean. Yep, currently space is slightly used during = compile time,=20 so its lookup can be always deferred until runtime. But in the nearest = future, all properties=20 will be fetched directly from space (at compile time), so there will be = no need to make additional space lookup at runtime, if we load it into VDBE during = compile time.=20 Patches on process_rw() and updates from this one are below. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Originally, process_rw() function implements internal logic (such as txn or metrics routine) and calls DML executor. However, it isn't available for other modules (i.e. declared as static). On the other hand, it seems to be very useful for SQL, since it allows to reduce call stack and get rid of redundant space lookup. Thus, lets rename it to 'box_process_rw()' and declare in box.h --- src/box/box.cc | 11 ++++++----- src/box/box.h | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/box/box.cc b/src/box/box.cc index a3bbdfce8..12c662dd8 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -160,8 +160,9 @@ box_check_memtx_min_tuple_size(ssize_t = memtx_min_tuple_size) "specified value is out of bounds"); } =20 -static int -process_rw(struct request *request, struct space *space, struct tuple = **result) +int +box_process_rw(struct request *request, struct space *space, + struct tuple **result) { assert(iproto_type_is_dml(request->type)); rmean_collect(rmean_box, request->type, 1); @@ -279,7 +280,7 @@ apply_row(struct xstream *stream, struct xrow_header = *row) struct request request; xrow_decode_dml_xc(row, &request, = dml_request_key_map(row->type)); struct space *space =3D space_cache_find_xc(request.space_id); - if (process_rw(&request, space, NULL) !=3D 0) { + if (box_process_rw(&request, space, NULL) !=3D 0) { say_error("error applying row: %s", = request_str(&request)); diag_raise(); } @@ -821,7 +822,7 @@ boxk(int type, uint32_t space_id, const char = *format, ...) struct space *space =3D space_cache_find(space_id); if (space =3D=3D NULL) return -1; - return process_rw(&request, space, NULL); + return box_process_rw(&request, space, NULL); } =20 int @@ -893,7 +894,7 @@ box_process1(struct request *request, box_tuple_t = **result) return -1; if (!space->def->opts.temporary && box_check_writable() !=3D 0) return -1; - return process_rw(request, space, result); + return box_process_rw(request, space, result); } =20 int diff --git a/src/box/box.h b/src/box/box.h index c9b5aad01..e337cb0ee 100644 --- a/src/box/box.h +++ b/src/box/box.h @@ -50,6 +50,7 @@ struct xrow_header; struct obuf; struct ev_io; struct auth_request; +struct space; =20 /* * Initialize box library @@ -379,15 +380,29 @@ box_sequence_reset(uint32_t seq_id); /** \endcond public */ =20 /** - * The main entry point to the + * Used to be entry point to the * Box: callbacks into the request processor. * These are function pointers since they can * change when entering/leaving read-only mode * (master->slave propagation). + * However, it makes space lookup. If space is already obtained, + * one can simply use internal box_process_rw(). */ int box_process1(struct request *request, box_tuple_t **result); =20 +/** + * Execute request on given space. + * + * \param request Request to be executed + * \param space Space to be updated + * \param result Result of executed request + * \retval 0 in success, -1 otherwise + */ +int +box_process_rw(struct request *request, struct space *space, + struct tuple **result); + int boxk(int type, uint32_t space_id, const char *format, ...); =20 --=20 2.15.1 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D /* * The function assumes the cursor is open on _schema. - * Increment max_id and store updated tuple in the cursor - * object. + * Increment max_id and store updated value it output parameter. */ -int tarantoolSqlite3IncrementMaxid(BtCursor *pCur) +int tarantoolSqlite3IncrementMaxid(uint64_t *space_max_id) { - assert(pCur->curFlags & BTCF_TaCursor); /* ["max_id"] */ static const char key[] =3D { (char)0x91, /* MsgPack array(1) */ @@ -1035,6 +1050,8 @@ int tarantoolSqlite3IncrementMaxid(BtCursor *pCur) =20 struct tuple *res =3D NULL; int rc; + struct space *space_schema =3D space_by_id(BOX_SCHEMA_ID); + assert(space_schema !=3D NULL); struct request request; memset(&request, 0, sizeof(request)); request.tuple =3D ops; @@ -1042,18 +1059,14 @@ int tarantoolSqlite3IncrementMaxid(BtCursor = *pCur) request.key =3D key; request.key_end =3D key + sizeof(key); request.type =3D IPROTO_UPDATE; - request.space_id =3D pCur->space->def->id; - rc =3D box_process_rw(&request, pCur->space, &res); + request.space_id =3D space_schema->def->id; + rc =3D box_process_rw(&request, space_schema, &res); if (rc !=3D 0 || res =3D=3D NULL) { return SQL_TARANTOOL_ERROR; } - if (pCur->last_tuple !=3D NULL) { - box_tuple_unref(pCur->last_tuple); - } - box_tuple_ref(res); - pCur->last_tuple =3D res; - pCur->eState =3D CURSOR_VALID; - return SQLITE_OK; + rc =3D tuple_field_u64(res, 1, space_max_id); + (*space_max_id)++; + return rc =3D=3D 0 ? SQLITE_OK : SQL_TARANTOOL_ERROR; } =20 /* @@ -1633,25 +1646,20 @@ int tarantoolSqlite3EphemeralGetMaxId(BtCursor = *pCur, uint32_t fieldno, } =20 /** - * Extract maximum integer value from: - * @param index space_id - * @param index_id - * @param field number fieldno - * @param[out] fetched value in max_id + * Extract next id from _sequence space. + * If index is empty - return 0 in max_id and success status * + * @param[out] max_id Fetched value. * @retval 0 on success, -1 otherwise. - * - * If index is empty - return 0 in max_id and success status */ int -tarantoolSqlGetMaxId(BtCursor *cur, uint32_t fieldno, - uint64_t *max_id) +tarantoolSqlNextSeqId(uint64_t *max_id) { char key[16]; struct tuple *tuple; char *key_end =3D mp_encode_array(key, 0); - if (box_index_max(cur->space->def->id, cur->index->def->iid, - key, key_end, &tuple) !=3D 0) + if (box_index_max(BOX_SEQUENCE_ID, 0 /* PK */, key, + key_end, &tuple) !=3D 0) return -1; =20 /* Index is empty */ @@ -1660,5 +1668,5 @@ tarantoolSqlGetMaxId(BtCursor *cur, uint32_t = fieldno, return 0; } =20 - return tuple_field_u64(tuple, fieldno, max_id); + return tuple_field_u64(tuple, 0, max_id); } diff --git a/src/box/sql/build.c b/src/box/sql/build.c index d5b0f6001..db7ffe24e 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2018,7 +2018,8 @@ sqlite3EndTable(Parse * pParse, /* Parse context = */ sqlite3OpenTable(pParse, iCursor, sys_sequence, OP_OpenWrite); reg_seq_id =3D ++pParse->nMem; - sqlite3VdbeAddOp3(v, OP_NextId, iCursor, 0, = reg_seq_id); + sqlite3VdbeAddOp3(v, OP_NextSequenceId, iCursor, = 0, + reg_seq_id); =20 reg_seq_record =3D = emitNewSysSequenceRecord(pParse, = reg_seq_id, --- a/src/box/sql/tarantoolInt.h +++ b/src/box/sql/tarantoolInt.h @@ -77,6 +77,7 @@ int tarantoolSqlite3Count(BtCursor * pCur, i64 * = pnEntry); int tarantoolSqlite3Insert(BtCursor * pCur); int tarantoolSqlite3Replace(BtCursor * pCur); int tarantoolSqlite3Delete(BtCursor * pCur, u8 flags); +int sql_delete_by_key(struct space *space, char *key, uint32_t = key_size); int tarantoolSqlite3ClearTable(struct space *space); =20 /* Rename table pTab with zNewName by inserting new tuple to _space. @@ -112,11 +113,10 @@ int tarantoolSqlite3IdxKeyCompare(BtCursor * pCur, = UnpackedRecord * pUnpacked, int *res); =20 /* - * The function assumes the cursor is open on _schema. - * Increment max_id and store updated tuple in the cursor - * object. + * The function assumes to be applied on _schema space. + * Increment max_id and store updated id in given argument. */ -int tarantoolSqlite3IncrementMaxid(BtCursor * pCur); +int tarantoolSqlite3IncrementMaxid(uint64_t *space_max_id); =20 /* * Render "format" array for _space entry. @@ -147,8 +147,6 @@ int tarantoolSqlite3MakeIdxParts(Index * index, void = *buf); int tarantoolSqlite3MakeIdxOpts(Index * index, const char *zSql, void = *buf); =20 /* - * Fetch maximum value from ineger column number `fieldno` of = space_id/index_id - * Return 0 on success, -1 otherwise + * Fetch next id from _sequence space. */ -int tarantoolSqlGetMaxId(BtCursor *cur, uint32_t fieldno, - uint64_t * max_id); +int tarantoolSqlNextSeqId(uint64_t *max_id); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 0ae682f9e..3d9ac8e26 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3830,28 +3830,16 @@ case OP_Sequence: { /* out2 */ break; } =20 -/* Opcode: NextId P1 P2 P3 * * - * Synopsis: r[P3]=3Dget_max(space_index[P1]{Column[P2]}) +/* Opcode: NextSequenceId * P2 * * * + * Synopsis: r[P2]=3Dget_max(_sequence) * - * Get next Id of the table. P1 is a table cursor, P2 is column - * number. Return in P3 maximum id found in provided column, + * Get next Id of the _sequence space. + * Return in P2 maximum id found in _sequence, * incremented by one. - * - * This opcode is Tarantool specific and will segfault in case - * of SQLite cursor. */ -case OP_NextId: { /* out3 */ - VdbeCursor *pC; /* The VDBE cursor */ - int p2; /* Column number, which stores the id */ - pC =3D p->apCsr[pOp->p1]; - p2 =3D pOp->p2; - pOut =3D &aMem[pOp->p3]; - - /* This opcode is Tarantool specific. */ - assert(pC->uc.pCursor->curFlags & BTCF_TaCursor); - - tarantoolSqlGetMaxId(pC->uc.pCursor, p2, - (uint64_t *) &pOut->u.i); +case OP_NextSequenceId: { + pOut =3D &aMem[pOp->p2]; + tarantoolSqlNextSeqId((uint64_t *) &pOut->u.i); =20 pOut->u.i +=3D 1; pOut->flags =3D MEM_Int; @@ -4495,6 +4483,85 @@ case OP_IdxInsert: { /* in2 */ break; } =20 +/* Opcode: SInsert P1 P2 * * P5 + * Synopsis: space id =3D P1, key =3D r[P2] + * + * This opcode is used only during DDL routine. + * In contrast to ordinary insertion, insertion to system spaces + * such as _space or _index will lead to schema changes. + * Thus, usage of space pointers is going to be impossible, + * as far as pointers can be expired since compilation time. + * + * If P5 is set to OPFLAG_NCHANGE, account overall changes + * made to database. + */ +case OP_SInsert: { + assert(pOp->p1 > 0); + assert(pOp->p2 >=3D 0); + + pIn2 =3D &aMem[pOp->p2]; + struct space *space =3D space_by_id(pOp->p1); + assert(space !=3D NULL); + assert(space_is_system(space)); + /* Create surrogate cursor to pass to SQL bindings. */ + BtCursor surrogate_cur; + surrogate_cur.space =3D space; + surrogate_cur.key =3D pIn2->z; + surrogate_cur.nKey =3D pIn2->n; + surrogate_cur.curFlags =3D BTCF_TaCursor; + rc =3D tarantoolSqlite3Insert(&surrogate_cur); + if (rc) + goto abort_due_to_error; + if (pOp->p5 & OPFLAG_NCHANGE) + p->nChange++; + break; +} + +/* Opcode: SDelete P1 P2 * * P5 + * Synopsis: space id =3D P1, key =3D r[P2] + * + * This opcode is used only during DDL routine. + * Delete entry with given key from system space. + * + * If P5 is set to OPFLAG_NCHANGE, account overall changes + * made to database. + */ +case OP_SDelete: { + assert(pOp->p1 > 0); + assert(pOp->p2 >=3D 0); + + pIn2 =3D &aMem[pOp->p2]; + struct space *space =3D space_by_id(pOp->p1); + assert(space !=3D NULL); + assert(space_is_system(space)); + rc =3D sql_delete_by_key(space, pIn2->z, pIn2->n); + if (rc) + goto abort_due_to_error; + if (pOp->p5 & OPFLAG_NCHANGE) + p->nChange++; + break; +} + +/* Opcode: SIDtoPtr P1 P2 * * * + * Synopsis: space id =3D P1, space[out] =3D r[P2] + * + * This opcode makes look up by space id and save found space + * into register, specified by the content of register P2. + * Such trick is needed during DLL routine, since schema may + * change and pointers become expired. + */ +case OP_SIDtoPtr: { + assert(pOp->p1 > 0); + assert(pOp->p2 >=3D 0); + + pIn2 =3D out2Prerelease(p, pOp); + struct space *space =3D space_by_id(pOp->p1); + assert(space !=3D NULL); + pIn2->u.p =3D (void *) space; + pIn2->flags =3D MEM_Ptr; + break; +} + /* Opcode: IdxDelete P1 P2 P3 * * * Synopsis: key=3Dr[P2@P3] * @@ -5424,22 +5491,19 @@ case OP_Init: { /* jump */ =20 /* Opcode: IncMaxid P1 * * * * * - * The cursor (P1) should be open on _schema. - * Increment the max_id (max space id) and store updated tuple in the - * cursor. + * Increment the max_id from _schema (max space id) + * and store updated id in register specified by first operand. + * It is system opcode and must be used only during DDL routine. */ case OP_IncMaxid: { - VdbeCursor *pC; - - assert(pOp->p1>=3D0 && pOp->p1nCursor); - pC =3D p->apCsr[pOp->p1]; - assert(pC !=3D 0); + assert(pOp->p1 > 0); + pOut =3D &aMem[pOp->p1]; =20 - rc =3D tarantoolSqlite3IncrementMaxid(pC->uc.pCursor); + rc =3D tarantoolSqlite3IncrementMaxid((uint64_t*) &pOut->u.i); if (rc!=3DSQLITE_OK) { goto abort_due_to_error; } - pC->nullRow =3D 0; + pOut->flags =3D MEM_Int; break; } =20 --=20 2.15.1 --Apple-Mail=_1CC5F354-A9F6-48FE-B883-6A3353129CF4 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
On 22 Mar 2018, at 16:09, v.shpilevoy@tarantool.org wrote:



22 =D0=BC=D0=B0=D1=80=D1=82= =D0=B0 2018 =D0=B3., =D0=B2 15:23, n.pettik <korablev@tarantool.org> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0= =B0=D0=BB(=D0=B0):


On 22 Mar 2018, at 14:42, v.shpilevoy@tarantool.org wrote:

See below 6 comments.

21 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2018 = =D0=B3., =D0=B2 2:48, Nikita Pettik <korablev@tarantool.org> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0= =B0=D0=BB(=D0=B0):

Since it is impossible = to use space pointers during execution of DDL
(after any = DDL schema may change and pointers fetched at compile time
can become expired), special opcodes to operate on system = spaces have
been introduced: OP_SInsert and OP_SDelete. = They take space id as
an argument and make space lookup = each time before insertion or deletion.
However, sometimes = it is still required to iterate through space (e.g.
to = satisfy WHERE clause) during DDL. As far as now cursors rely on
pointers to space, it is required to convert space id to = space pointer
during VDBE execution. Hence, another opcode = is added: OP_SIDtoPtr.
Finally, existing opcodes, which = are invoked only during DDL, have been
also refactored.

Part of #3252
---
src/box/sql.c =             &n= bsp;|  51 ++++++++++++-------
src/box/sql/opcodes.c =      |  45 +++++++++--------
src/box/sql/opcodes.h      | =  53 ++++++++++----------
src/box/sql/tarantoolInt.h | =  11 ++---
src/box/sql/vdbe.c =         | 121 = ++++++++++++++++++++++++++++++++++-----------
5 files = changed, 182 insertions(+), 99 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index = e943131ae..db4c7e7ea 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
+int
+sql_delete_by_key(struct space *space, char *key, uint32_t = key_size)
+{
struct request request;
= memset(&request, 0, sizeof(request));
= request.type =3D IPROTO_DELETE;
= request.key =3D key;
request.key_end =3D key + = key_size;
- request.space_id =3D = pCur->space->def->id;
- rc =3D = sql_execute_dml(&request, pCur->space);
+ = request.space_id =3D space->def->id;
+ int rc =3D = sql_execute_dml(&request, space);

1. Why do you need sql_execute_dml? It does exactly the = same as process_rw, but process_rw can also return a tuple. And this = process_rw feature is needed for the patch on last inserted tuple = returning.

I need it as an analogue of process_rw(), since process_rw() = is unavailable 
via public BOX interface = (declared as static). If you approve to make it public,
where should I place it and should I rename it?
Or, for instance, make sql_execute_dml be complete copy of = process_rw()?
It is unlikely to be useful somewhere = except for SQL (and box internals surely).
Initial = indent was to reduce overhead while calling box functions from SQL = bindings (sql.c):
ordinary call stack looks like: = box_insert() -> box_process1() -> space lookup + = process_rw().
In such scheme we also process = redundant space lookup since we already have struct space * in = cursor.
(Patch where I introduced this function = isn=E2=80=99t in trunk, so I can easily fix = it).

Yes, I very like the removal = of box_process1. I just do not like code duplicate. But also I can not = see a place for declaration of process_rw in any header. So lets make it = extern "C" non-static in box.cc, and declare it as extern in = sql.c.

Ok, I=E2=80=99= m attaching patch to email (since it belongs to another patch-set: = np/gh-3122-remove-pgnoRoot)




@@ = -1088,20 +1107,16 @@ int tarantoolSqlite3IncrementMaxid(BtCursor = *pCur)
request.ops =3D ops;
= request.ops_end =3D ops + sizeof(ops);
= request.type =3D IPROTO_UPSERT;
- = request.space_id =3D pCur->space->def->id;
- = rc =3D sql_execute_dml(&request, pCur->space);
+ = request.space_id =3D BOX_SCHEMA_ID;
+ rc =3D = sql_execute_dml(&request, space_schema);

2. If you use process_rw, then you do not = need iterator to get the previous max_id and increment it - you just do = process_rw, get result and do tuple_field_u64(result, 1, = space_max_id);
Suppose, this will be fixed, = if you will use process_rw, as described = above.

Fixed (I = am attaching changes at the end of mail).



if (rc !=3D 0) {
= = iterator_delete(it);
return SQL_TARANTOOL_ERROR;
= }
- if (pCur->last_tuple !=3D = NULL) {
- = box_tuple_unref(pCur->last_tuple);
- }
- = box_tuple_ref(res);
- pCur->last_tuple =3D res;
- = pCur->eState =3D CURSOR_VALID;
+ rc =3D = tuple_field_u64(res, 1, space_max_id);
+ = (*space_max_id)++;
iterator_delete(it);
- = return SQLITE_OK;
+ return rc  =3D=3D 0 ? = SQLITE_OK : SQL_TARANTOOL_ERROR;
}

/*
@@ -1687,14 +1702,12 @@ int = tarantoolSqlite3EphemeralGetMaxId(BtCursor *pCur, uint32_t fieldno,
* If index is empty - return 0 in max_id and success = status
*/
int
-tarantoolSqlGetMaxId(BtCursor *cur, uint32_t fieldno,
- = =      uint6= 4_t *max_id)
+tarantoolSqlGetMaxId(uint32_t space_id, = uint64_t *max_id)

3. After changing a function = signature, please, fix a comments too. And why this function is sure, = that the max id stored in the first column? And why it can not be used = for secondary indexes?

It might, but it is only applied for _sequence space during = DDL routine
(and this is also answer for the first = question: format of system space doesn=E2=80=99t change = frequently),
so I decided to remove cursor/index = overhead (as a part of overall DDL refactoring).
Probably, I should also rename this opcode to make it more = clear.

Yes, lets rename it, and = remove space_id argument, if it is always used for _sequence = space.

Done (I am = attaching changes at the end of mail).



+/* = Opcode: SInsert P1 P2 * * P5
+ * Synopsis: space id =3D = P1, key =3D r[P2]
+ *
+ * This opcode is = used only during DML routine.

4. Do you mean DDL instead of = DML?
Hm?

Fixed (at both places).



+ * In contrast to ordinary insertion, = insertion to system spaces
+ * such as _space or _index = will lead to schema changes.
+ * Thus, usage of space = pointers is going to be impossible,
+ * as far as pointers = can be expired since compilation time.
+ *
+ = * If P5 is set to OPFLAG_NCHANGE, account overall changes
+ = * made to database.
+ */
+case OP_SInsert: = {
+ assert(pOp->p1 > 0);
+
+/* Opcode: SIDtoPtr P1 = P2 * * *
+ * Synopsis: space id =3D P1, space[out] =3D = r[P2]
+ *
+ * This opcode makes look up by = space id and save found space
+ * into register, specified = by the content of register P2.
+ * Such trick is needed = during DML routine, since schema may
+ * change and = pointers become expired.
+ */
+case = OP_SIDtoPtr: {

6. It seems to be unused in = the patch. Can you use it for all these things, introduced by the = previous patch?
struct space *space =3D = space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
assert(space !=3D = NULL);
sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, = space_ptr_reg, 0, (void *) space);
sqlite3VdbeAddOp3(v, = OP_OpenRead, iIdxCur, pIdx->tnum, space_ptr_reg);

Using OP_SIDtoPtr you already = in this patch can defer space_by_id lookup.

It is used in the next patch. I don=E2=80=99t want to make a = lot of diffs, since introducing of any opcodes
leads = to auto-generating of opcodes.c/h and as a result to vast amount of = redundant diff.
Ok, then lets leave it in = this patch.

As= for your example, during SQL DDL we need to execute queries such = as:
'DELETE FROM BOX_SEQUENCE WHERE name =3D = zName;=E2=80=99
And =E2=80=98name=E2=80=99 is not a = key. Hence, we have to open cursor and iterate through the = space.
But if we used pointers to spaces which were = fetched at compile state,
they would become expired = at VDBE runtime:
after first stage of DDL (e.g. = deletion from _trigger), schema may change.
Moreover,= we are going to support syntax like: =E2=80=98CREATE TABLE AS SELECT = =E2=80=A6=E2=80=99,
where DDL and DML are mixing. = Thus we need facility to make space lookup at VDBE = runtime.

I agree, that we must have = this opcode, but why it can not be used for DML too? In my example you = at compile time get space *, store it by OP_LoadPtr, and then use to open = iterator (but I can not see where - I commented this in a previous patch = review), but for DML you can replace compile lookups on runtime lookups = using OP_SIDtoPtr. And it also solves the problem, that a = prepared VDBE statement (which in a future we will store for a long = time) must store space * in opcodes - we can use OP_SIDtoPtr and do not = store it until execution. Sorry, can not explain more clear. We can also = discuss it face-to-face.

Now I get what you mean. Yep, currently space is = slightly used during compile time, 
so its lookup can be = always deferred until runtime. But in the nearest future, all = properties 
will be fetched directly from space (at = compile time), so there will be no need to make
additional = space lookup at runtime, if we load it into VDBE during compile = time. 

Patches on process_rw() = and updates from this one are below.

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D

Originally, = process_rw() function implements internal logic (such as = txn
or metrics routine) and calls DML executor. However, it = isn't available
for other modules (i.e. declared as static). = On the other hand, it seems
to be very useful for SQL, since = it allows to reduce call stack and get
rid of redundant space = lookup.  Thus, lets rename it to
'box_process_rw()' and = declare in box.h
---
 src/box/box.cc | 11 = ++++++-----
 src/box/box.h  | 17 = ++++++++++++++++-
 2 files changed, 22 insertions(+), 6 = deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index = a3bbdfce8..12c662dd8 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -160,8 +160,9 = @@ box_check_memtx_min_tuple_size(ssize_t = memtx_min_tuple_size)
   "specified value = is out of bounds");
 }
 
-static = int
-process_rw(struct request *request, struct space *space, = struct tuple **result)
+int
+box_process_rw(struct = request *request, struct space *space,
+   =     struct tuple = **result)
 {
  = assert(iproto_type_is_dml(request->type));
  = rmean_collect(rmean_box, request->type, 1);
@@ = -279,7 +280,7 @@ apply_row(struct xstream *stream, struct xrow_header = *row)
  struct request = request;
  xrow_decode_dml_xc(row, = &request, dml_request_key_map(row->type));
  struct = space *space =3D space_cache_find_xc(request.space_id);
- if = (process_rw(&request, space, NULL) !=3D 0) {
+ if = (box_process_rw(&request, space, NULL) !=3D 0) = {
  say_error("error applying = row: %s", request_str(&request));
  = diag_raise();
  }
@@ -821,7 +822,7 @@ = boxk(int type, uint32_t space_id, const char *format, = ...)
  struct space *space =3D = space_cache_find(space_id);
  if (space =3D=3D = NULL)
  return = -1;
- = return process_rw(&request, space, NULL);
+ return = box_process_rw(&request, space, = NULL);
 }
 
 int
@@= -893,7 +894,7 @@ box_process1(struct request *request, box_tuple_t = **result)
  return = -1;
  if = (!space->def->opts.temporary && box_check_writable() !=3D = 0)
  return = -1;
- = return process_rw(request, space, result);
+ return = box_process_rw(request, space, = result);
 }
 
 int
= diff --git a/src/box/box.h b/src/box/box.h
index = c9b5aad01..e337cb0ee 100644
--- a/src/box/box.h
+++ = b/src/box/box.h
@@ -50,6 +50,7 @@ struct = xrow_header;
 struct obuf;
 struct = ev_io;
 struct auth_request;
+struct = space;
 
 /*
  * Initialize = box library
@@ -379,15 +380,29 @@ box_sequence_reset(uint32_t = seq_id);
 /** \endcond public = */
 
 /**
- * The main entry = point to the
+ * Used to be entry point to = the
  * Box: callbacks into the request = processor.
  * These are function pointers since they = can
  * change when entering/leaving read-only = mode
  * (master->slave propagation).
+ * = However, it makes space lookup. If space is already = obtained,
+ * one can simply use internal = box_process_rw().
  = */
 int
 box_process1(struct request = *request, box_tuple_t = **result);
 
+/**
+ * Execute request = on given space.
+ *
+ * \param request Request to be = executed
+ * \param space Space to be updated
+ * = \param result Result of executed request
+ * \retval 0 in = success, -1 otherwise
+ = */
+int
+box_process_rw(struct request *request, = struct space *space,
+       struct = tuple = **result);
+
 int
 boxk(int = type, uint32_t space_id, const char *format, = ...);
 
-- 
2.15.1

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D


 /*
  * The function assumes the cursor is open on = _schema.
- * Increment max_id and store updated = tuple in the cursor
- * object.
+ * Increment max_id and store updated value it output = parameter.
  */
-int = tarantoolSqlite3IncrementMaxid(BtCursor *pCur)
+int = tarantoolSqlite3IncrementMaxid(uint64_t *space_max_id)
 {
- assert(pCur->curFlags & = BTCF_TaCursor);
  /* ["max_id"] */
 = static const char key[] =3D {
 = (char)0x91, /* MsgPack array(1) */
@@ -1035,6 +1050,8 @@ int = tarantoolSqlite3IncrementMaxid(BtCursor *pCur)
 
  struct = tuple *res =3D NULL;
  int = rc;
+ struct space *space_schema =3D = space_by_id(BOX_SCHEMA_ID);
+ = assert(space_schema !=3D NULL);
  struct = request request;
  memset(&request, 0, = sizeof(request));
  = request.tuple =3D ops;
@@ -1042,18 +1059,14 = @@ int tarantoolSqlite3IncrementMaxid(BtCursor *pCur)
 = request.key =3D key;
  = request.key_end =3D key + sizeof(key);
 = request.type =3D IPROTO_UPDATE;
- = request.space_id =3D pCur->space->def->id;
- = rc =3D box_process_rw(&request, pCur->space, = &res);
+ request.space_id =3D = space_schema->def->id;
+ rc =3D = box_process_rw(&request, space_schema, &res);
 = if (rc !=3D 0 || res =3D=3D NULL) {
 = return SQL_TARANTOOL_ERROR;
 = }
- if (pCur->last_tuple !=3D = NULL) {
- = box_tuple_unref(pCur->last_tuple);
- = }
- box_tuple_ref(res);
- = pCur->last_tuple =3D res;
- = pCur->eState =3D CURSOR_VALID;
- return = SQLITE_OK;
+ rc =3D tuple_field_u64(res, 1, = space_max_id);
+ (*space_max_id)++;
+ = return rc  =3D=3D 0 ? SQLITE_OK : = SQL_TARANTOOL_ERROR;
 }
 
 /*
@@ = -1633,25 +1646,20 @@ int tarantoolSqlite3EphemeralGetMaxId(BtCursor = *pCur, uint32_t fieldno,
 }
 
 /**
- = * Extract maximum integer value from:
- * @param = index space_id
- * @param index_id
- * @param field number fieldno
- * = @param[out] fetched value in max_id
+ * Extract = next id from _sequence space.
+ * If index is empty = - return 0 in max_id and success status
  = *
+ * @param[out] max_id Fetched value.
  * @retval 0 on success, -1 otherwise.
- *
- * If index is empty - return 0 in = max_id and success status
  */
 int
-tarantoolSqlGetMaxId(BtCursor = *cur, uint32_t fieldno,
- =     uint64_t *max_id)
+tarantoolSqlNextSeqId(uint64_t *max_id)
 {
  char = key[16];
  struct tuple *tuple;
 = char *key_end =3D mp_encode_array(key, 0);
- = if (box_index_max(cur->space->def->id, = cur->index->def->iid,
- =  key, key_end, &tuple) !=3D 0)
+ = if (box_index_max(BOX_SEQUENCE_ID, 0 /* PK */, key,
+ =  key_end, &tuple) !=3D 0)
 = return -1;
 
 = /* Index is empty  */
@@ = -1660,5 +1668,5 @@ tarantoolSqlGetMaxId(BtCursor *cur, uint32_t = fieldno,
  return 0;
 = }
 
- return = tuple_field_u64(tuple, fieldno, max_id);
+ return = tuple_field_u64(tuple, 0, max_id);
 }
diff --git a/src/box/sql/build.c = b/src/box/sql/build.c
index d5b0f6001..db7ffe24e = 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2018,7 = +2018,8 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
 = sqlite3OpenTable(pParse, iCursor, = sys_sequence,
  = OP_OpenWrite);
  reg_seq_id =3D = ++pParse->nMem;
- = sqlite3VdbeAddOp3(v, OP_NextId, iCursor, 0, = reg_seq_id);
+ = sqlite3VdbeAddOp3(v, OP_NextSequenceId, iCursor, 0,
+ =  reg_seq_id);
 
  = reg_seq_record =3D emitNewSysSequenceRecord(pParse,
 = =  reg_seq_id,

--- a/src/box/sql/tarantoolInt.h
+++ = b/src/box/sql/tarantoolInt.h
@@ -77,6 +77,7 @@ int = tarantoolSqlite3Count(BtCursor * pCur, i64 * pnEntry);
 int tarantoolSqlite3Insert(BtCursor * pCur);
 int tarantoolSqlite3Replace(BtCursor * pCur);
 int tarantoolSqlite3Delete(BtCursor * pCur, u8 = flags);
+int sql_delete_by_key(struct space *space, = char *key, uint32_t key_size);
 int = tarantoolSqlite3ClearTable(struct space *space);
 
 /* Rename table pTab with = zNewName by inserting new tuple to _space.
@@ = -112,11 +113,10 @@ int tarantoolSqlite3IdxKeyCompare(BtCursor * pCur, = UnpackedRecord * pUnpacked,
  =  int *res);
 
 /*
- * The function assumes the = cursor is open on _schema.
- * Increment max_id and = store updated tuple in the cursor
- * = object.
+ * The function assumes to be applied on = _schema space.
+ * Increment max_id and store = updated id in given argument.
  */
-int tarantoolSqlite3IncrementMaxid(BtCursor * = pCur);
+int tarantoolSqlite3IncrementMaxid(uint64_t = *space_max_id);
 
 /*
  * Render "format" array = for _space entry.
@@ -147,8 +147,6 @@ int = tarantoolSqlite3MakeIdxParts(Index * index, void *buf);
 int tarantoolSqlite3MakeIdxOpts(Index * index, const = char *zSql, void *buf);
 
 /*
- * Fetch maximum value from = ineger column number `fieldno` of space_id/index_id
-= * Return 0 on success, -1 otherwise
+ * Fetch next = id from _sequence space.
  */
-int tarantoolSqlGetMaxId(BtCursor *cur, uint32_t = fieldno,
- uint64_t * = max_id);
+int tarantoolSqlNextSeqId(uint64_t = *max_id);
diff --git a/src/box/sql/vdbe.c = b/src/box/sql/vdbe.c
index 0ae682f9e..3d9ac8e26 = 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3830,28 = +3830,16 @@ case OP_Sequence: {           /* = out2 */
  break;
 }
 
-/* = Opcode: NextId P1 P2 P3 * *
- * Synopsis: = r[P3]=3Dget_max(space_index[P1]{Column[P2]})
+/* = Opcode: NextSequenceId * P2 * * *
+ * Synopsis: = r[P2]=3Dget_max(_sequence)
  *
- * Get next Id of the table. P1 is a table cursor, P2 is = column
- * number. Return in P3 maximum id found in = provided column,
+ * Get next Id of the _sequence = space.
+ * Return in P2 maximum id found in = _sequence,
  * incremented by one.
- *
- * This opcode is Tarantool = specific and will segfault in case
- * of SQLite = cursor.
  */
-case = OP_NextId: {     /* out3 */
- = VdbeCursor *pC;    /* The VDBE cursor */
- = int p2;            /* Column = number, which stores the id */
- pC =3D = p->apCsr[pOp->p1];
- p2 =3D = pOp->p2;
- pOut =3D = &aMem[pOp->p3];
-
- /* This = opcode is Tarantool specific.  */
- = assert(pC->uc.pCursor->curFlags & = BTCF_TaCursor);
-
- = tarantoolSqlGetMaxId(pC->uc.pCursor, p2,
- =     (uint64_t *) &pOut->u.i);
+case OP_NextSequenceId: {
+ pOut =3D = &aMem[pOp->p2];
+ = tarantoolSqlNextSeqId((uint64_t *) &pOut->u.i);
 
  = pOut->u.i +=3D 1;
  = pOut->flags =3D MEM_Int;
@@ -4495,6 = +4483,85 @@ case OP_IdxInsert: {        /* in2 = */
  break;
 }
 
+/* = Opcode: SInsert P1 P2 * * P5
+ * Synopsis: space id = =3D P1, key =3D r[P2]
+ *
+ * = This opcode is used only during DDL routine.
+ * In = contrast to ordinary insertion, insertion to system spaces
+ * such as _space or _index will lead to schema = changes.
+ * Thus, usage of space pointers is going = to be impossible,
+ * as far as pointers can be = expired since compilation time.
+ *
+ * If P5 is set to OPFLAG_NCHANGE, account overall = changes
+ * made to database.
+ = */
+case OP_SInsert: {
+ = assert(pOp->p1 > 0);
+ = assert(pOp->p2 >=3D 0);
+
+ = pIn2 =3D &aMem[pOp->p2];
+ struct = space *space =3D space_by_id(pOp->p1);
+ = assert(space !=3D NULL);
+ = assert(space_is_system(space));
+ /* Create = surrogate cursor to pass to SQL bindings. */
+ BtCursor = surrogate_cur;
+ surrogate_cur.space =3D = space;
+ surrogate_cur.key =3D = pIn2->z;
+ surrogate_cur.nKey =3D = pIn2->n;
+ surrogate_cur.curFlags =3D = BTCF_TaCursor;
+ rc =3D = tarantoolSqlite3Insert(&surrogate_cur);
+ if = (rc)
+ goto = abort_due_to_error;
+ if (pOp->p5 & = OPFLAG_NCHANGE)
+ = p->nChange++;
+ = break;
+}
+
+/* Opcode: SDelete P1 P2 * * P5
+ * = Synopsis: space id =3D P1, key =3D r[P2]
+ = *
+ * This opcode is used only during DDL = routine.
+ * Delete entry with given key from = system space.
+ *
+ * If P5 is = set to OPFLAG_NCHANGE, account overall changes
+ * = made to database.
+ */
+case = OP_SDelete: {
+ assert(pOp->p1 > = 0);
+ assert(pOp->p2 >=3D = 0);
+
+ pIn2 =3D = &aMem[pOp->p2];
+ struct = space *space =3D space_by_id(pOp->p1);
+ = assert(space !=3D NULL);
+ = assert(space_is_system(space));
+ rc =3D = sql_delete_by_key(space, pIn2->z, pIn2->n);
+ = if (rc)
+ goto = abort_due_to_error;
+ if (pOp->p5 & = OPFLAG_NCHANGE)
+ = p->nChange++;
+ = break;
+}
+
+/* Opcode: SIDtoPtr P1 P2 * * *
+ * = Synopsis: space id =3D P1, space[out] =3D r[P2]
+ = *
+ * This opcode makes look up by space id and = save found space
+ * into register, specified by = the content of register P2.
+ * Such trick is = needed during DLL routine, since schema may
+ * = change and pointers become expired.
+ */
+case OP_SIDtoPtr: {
+ = assert(pOp->p1 > 0);
+ = assert(pOp->p2 >=3D 0);
+
+ = pIn2 =3D out2Prerelease(p, pOp);
+ struct = space *space =3D space_by_id(pOp->p1);
+ = assert(space !=3D NULL);
+ = pIn2->u.p =3D (void *) space;
+ = pIn2->flags =3D MEM_Ptr;
+ = break;
+}
+
 /* Opcode: IdxDelete P1 P2 P3 * *
  * Synopsis: key=3Dr[P2@P3]
  = *
@@ -5424,22 +5491,19 @@ case OP_Init: {   =        /* jump */
 
 /* Opcode: IncMaxid P1 * * = * *
  *
- * The cursor = (P1) should be open on _schema.
- * Increment the = max_id (max space id) and store updated tuple in the
- * cursor.
+ * Increment the max_id = from _schema (max space id)
+ * and store updated = id in register specified by first operand.
+ * It = is system opcode and must be used only during DDL routine.
  */
 case OP_IncMaxid: = {
- VdbeCursor *pC;
-
- assert(pOp->p1>=3D0 = && pOp->p1<p->nCursor);
- pC =3D = p->apCsr[pOp->p1];
- assert(pC = !=3D 0);
+ assert(pOp->p1 > = 0);
+ pOut =3D = &aMem[pOp->p1];
 
- = rc =3D = tarantoolSqlite3IncrementMaxid(pC->uc.pCursor);
+ = rc =3D tarantoolSqlite3IncrementMaxid((uint64_t*) = &pOut->u.i);
  if = (rc!=3DSQLITE_OK) {
  = goto abort_due_to_error;
  = }
- pC->nullRow =3D 0;
+ = pOut->flags =3D MEM_Int;
  = break;
 }
 
-- 
2.15.1




= --Apple-Mail=_1CC5F354-A9F6-48FE-B883-6A3353129CF4--