On 22 Mar 2018, at 17:11, v.shpilevoy@tarantool.org wrote:

See below 3 comments.

21 марта 2018 г., в 2:48, Nikita Pettik <korablev@tarantool.org> написал(а):

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.

+ * 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?

+ /*
+  * 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?