> On 22 Mar 2018, at 17:11, v.shpilevoy@tarantool.org wrote: > > See below 3 comments. > >> 21 марта 2018 г., в 2:48, Nikita Pettik > написал(а): >> >> After new DDL SQL implementation has been introduced, OP_OpenWrite, >> OP_OpenRead and OP_ReopenIdx opcodes can be refactored. >> >> Firstly, if schema versions at compile time and runtime don't match, >> finish VDBE execution with appropriate message error. Exception is the >> situation when fifth pointer is set to OPFLAG_FRESH_PTR, which means >> that space pointer has been fetched during runtime right before that. >> >> Secondly, there is no need to fetch number of columns in index from >> KeyInfo: iterator yields the full tuple, so it would always be equal to >> the number of fields in a whole space. >> >> Finally, now we always can pass space pointer to these opcodes >> regardless of DML routine. In case of OP_ReopenIdx opcode space and >> index from given cursor is checked on equality to those given in >> arguments. If they match, this opcode will become no-op. >> --- >> src/box/sql/opcodes.c | 6 +- >> src/box/sql/opcodes.h | 6 +- >> src/box/sql/vdbe.c | 197 +++++++++++++++++--------------------------------- >> 3 files changed, 71 insertions(+), 138 deletions(-) >> >> -/* Opcode: OpenRead P1 P2 * P4 P5 >> - * Synopsis: root=P2 >> +/* Opcode: OpenRead P1 P2 PЗ P4 P5 > > 1. Seems like you wrote here a cyrillic letter 'З' instead of digit three. Holy sh… How did you manage to notice that? My font displays them almost identical.. I would call it ’next-level review’. Anyway thanks, fixed. > >> + * Synopsis: index id = P2, space ptr = P3 >> * >> - * Open a read-only cursor for the database table whose root page is >> - * P2 in a database file. >> - * Give the new cursor an identifier of P1. The P1 >> - * values need not be contiguous but all P1 values should be small integers. >> - * It is an error for P1 to be negative. >> + * Open a cursor for a space specified by pointer in P3 and index >> + * id in P2. Give the new cursor an identifier of P1. The P1 >> + * values need not be contiguous but all P1 values should be >> + * small integers. It is an error for P1 to be negative. >> * >> - * If P5!=0 then use the content of register P2 as the root page, not >> - * the value of P2 itself. >> + * The P4 value may be a pointer to a KeyInfo structure. >> + * If it is a pointer to a KeyInfo structure, then said structure >> + * defines the content and collatining sequence of the index >> + * being opened. Otherwise, P4 is NULL. >> * >> - * There will be a read lock on the database whenever there is an >> - * open cursor. If the database was unlocked prior to this instruction >> - * then a read lock is acquired as part of this instruction. A read >> - * lock allows other processes to read the database but prohibits >> - * any other process from modifying the database. The read lock is >> - * released when all cursors are closed. If this instruction attempts >> - * to get a read lock but fails, the script terminates with an >> - * SQLITE_BUSY error code. >> - * >> - * The P4 value may be either an integer (P4_INT32) or a pointer to >> - * a KeyInfo structure (P4_KEYINFO). If it is a pointer to a KeyInfo >> - * structure, then said structure defines the content and collating >> - * sequence of the index being opened. Otherwise, if P4 is an integer >> - * value, it is set to the number of columns in the table. >> - * >> - * See also: OpenWrite, ReopenIdx >> + * If schema has changed since compile time, VDBE ends execution >> + * with appropriate error message. The only exception is >> + * when P5 is set to OPFLAG_FRESH_PTR, which means that >> + * space pointer has been fetched in runtime right before >> + * this opcode. >> */ >> -/* Opcode: ReopenIdx P1 P2 * P4 P5 >> - * Synopsis: root=P2 >> +/* Opcode: ReopenIdx P1 P2 P3 P4 P5 >> + * Synopsis: index id = P2, space ptr = P3 >> * >> - * The ReopenIdx opcode works exactly like ReadOpen except that it first >> - * checks to see if the cursor on P1 is already open with a root page >> - * number of P2 and if it is this opcode becomes a no-op. In other words, >> - * if the cursor is already open, do not reopen it. >> + * The ReopenIdx opcode works exactly like ReadOpen except that > > 2. I can not find ReadOpen - possibly it is OP_OpenRead? Fixed. >> >> + /* >> + * Since Tarantool iterator yields the full tuple, >> + * we need a number of fields as wide as the table itself. >> + * Otherwise, not enough slots for row parser cache are >> + * allocated in VdbeCursor object. > > 3. Term 'yields' slightly confused. For the first moments it seems, that an iterator yields a fiber, not fields. Can you please find another word? Ok, replaced with ‘provides’. BTW, I didn’t come up with this word combination myself, just found it somewhere in source code...