sql: do not show IDs generated by trigger

Currently, if an INSERT was executed by a trigger during the
execution of a statement, if there were any generated identifiers,
they can be displayed as a result of the statement. This is
incorrect, since we are not able to divide the IDs obtained into
those that belong to the table mentioned in the statement and
those that do not belong to this table. This has now been fixed
by adding a new opcode and using it to retrieve and save the newly
generated identifiers.

For example:
box.execute('CREATE TABLE t1 (i INT PRIMARY KEY AUTOINCREMENT);')
box.execute('CREATE TABLE t2 (i INT PRIMARY KEY AUTOINCREMENT);')
box.execute('CREATE TRIGGER r AFTER INSERT ON t1 FOR EACH ROW '..
            'BEGIN INSERT INTO t2 VALUES (null); END')
box.execute('INSERT INTO t2 VALUES (100);')
box.execute('INSERT INTO t1 VALUES (NULL), (NULL), (NULL);')

Result should be:
---
- autoincrement_ids:
  - 1
  - 2
  - 3
  row_count: 3
...

Closes #4188

src/box/sequence.c

@@ -216,9 +216,6 @@ sequence_next(struct sequence *seq, int64_t *result)
 					  new_data) == light_sequence_end)
 			return -1;
 		*result = def->start;
-		if (txn_vdbe() != NULL &&
-		    vdbe_add_new_autoinc_id(txn_vdbe(), *result) != 0)
-			return -1;
 		return 0;
 	}
 	old_data = light_sequence_get(&sequence_data_index, pos);
@@ -253,9 +250,6 @@ sequence_next(struct sequence *seq, int64_t *result)
 				   new_data, &old_data) == light_sequence_end)
 		unreachable();
 	*result = value;
-	if (txn_vdbe() != NULL &&
-	    vdbe_add_new_autoinc_id(txn_vdbe(), value) != 0)
-		return -1;
 	return 0;
 overflow:
 	if (!def->cycle) {
@@ -368,3 +362,16 @@ sequence_data_iterator_create(void)
 	light_sequence_iterator_freeze(&sequence_data_index, &iter->iter);
 	return &iter->base;
 }
+
+int
+sequence_get_value(struct sequence *seq, int64_t *out_value)
+{
+	uint32_t key = seq->def->id;
+	uint32_t hash = sequence_hash(key);
+	uint32_t pos = light_sequence_find_key(&sequence_data_index, hash, key);
+	assert(pos != light_sequence_end);
+	struct sequence_data data = light_sequence_get(&sequence_data_index,
+						       pos);
+	*out_value = data.value;
+	return 0;
+}

src/box/sequence.h

@@ -157,9 +157,6 @@ sequence_next(struct sequence *seq, int64_t *result);
 int
 access_check_sequence(struct sequence *seq);
 
-int
-vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id);
-
 /**
  * Create an iterator over sequence data.
  *
@@ -170,6 +167,16 @@ vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id);
 struct snapshot_iterator *
 sequence_data_iterator_create(void);
 
+/**
+ * Get last element of given sequence.
+ *
+ * @param seq sequence to get value from.
+ * @param[out] out_value last element of sequence.
+ * @retval 0 on success, -1 on error.
+ */
+int
+sequence_get_value(struct sequence *seq, int64_t *out_value);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 

src/box/sql/insert.c

@@ -738,6 +738,19 @@ sqlInsert(Parse * pParse,	/* Parser context */
 		vdbe_emit_insertion_completion(v, space, regIns + 1,
 					       space->def->field_count,
 					       on_error);
+		/*
+		 * Save the newly generated autoincrement value to
+		 * VDBE context. We don't need to do so for
+		 * triggers in order to avoid mess of incremented
+		 * ids. After VDBE execution is finished, list of
+		 * autoincrement values is returned as a result of
+		 * query execution.
+		 */
+		if (autoinc_fieldno < UINT32_MAX &&
+		    pParse->triggered_space == NULL) {
+			sqlVdbeAddOp2(v, OP_SaveAutoincValue, space->def->id,
+				      regData + autoinc_fieldno);
+		}
 	}
 
 	/* Update the count of rows that are inserted

src/box/sql/vdbe.c

@@ -559,7 +559,7 @@ vdbe_autoinc_id_list(struct Vdbe *vdbe)
 	return &vdbe->autoinc_id_list;
 }
 
-int
+static int
 vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id)
 {
 	assert(vdbe != NULL);
@@ -1150,29 +1150,30 @@ case OP_String: {          /* out2 */
 }
 
 /* Opcode: NextAutoincValue P1 P2 * * *
- * Synopsis: r[P2] = next value from space sequence, which pageno is r[P1]
  *
- * Get next value from space sequence, which pageno is written into register
- * P1, write this value into register P2. If space doesn't exists (invalid
- * space_id or something else), raise an error. If space with
- * specified space_id doesn't have attached sequence, also raise an error.
+ * If the value in the register P2 is NULL, get the last value
+ * from the sequence that belongs to the space with id P1, and
+ * save this value in the VDBE. If the value in register in not
+ * NULL than this opcode is a no-op.
  */
-case OP_NextAutoincValue: {
+case OP_SaveAutoincValue: {
 	assert(pOp->p1 > 0);
 	assert(pOp->p2 > 0);
 
+	pIn2 = &p->aMem[pOp->p2];
+	if ((pIn2->flags & MEM_Null) == 0)
+		break;
+
 	struct space *space = space_by_id(pOp->p1);
 	if (space == NULL)
 		goto abort_due_to_error;
 
 	int64_t value;
-	struct sequence *sequence = space->sequence;
-	if (sequence == NULL || sequence_next(sequence, &value) != 0)
+	assert(space->sequence != NULL);
+	if (sequence_get_value(space->sequence, &value) != 0)
 		goto abort_due_to_error;
 
-	pOut = out2Prerelease(p, pOp);
-	pOut->flags = MEM_Int;
-	pOut->u.i = value;
+	vdbe_add_new_autoinc_id(p, value);
 
 	break;
 }

test/sql/iproto.result

@@ -552,8 +552,6 @@ cn:execute('insert into test values (null, 1)')
 ---
 - autoincrement_ids:
   - 127
-  - 1
-  - 2
   row_count: 1
 ...
 box.execute('create table test3 (id int primary key autoincrement)')

test/sql/triggers.result

@@ -551,3 +551,117 @@ box.execute("DROP TABLE t1;")
 ---
 - row_count: 1
 ...
+--
+-- gh-4188: Additional generated identifiers when INSERT is
+-- executed by triggers.
+--
+box.execute('CREATE TABLE t1 (i INT PRIMARY KEY AUTOINCREMENT);')
+---
+- row_count: 1
+...
+box.execute('CREATE TABLE t2 (i INT PRIMARY KEY AUTOINCREMENT);')
+---
+- row_count: 1
+...
+box.execute('CREATE TABLE t3 (i INT PRIMARY KEY AUTOINCREMENT);')
+---
+- row_count: 1
+...
+box.execute('CREATE TRIGGER r1 AFTER INSERT ON t2 FOR EACH ROW BEGIN INSERT INTO t1 VALUES (null); END')
+---
+- row_count: 1
+...
+box.execute('INSERT INTO t1 VALUES (100);')
+---
+- row_count: 1
+...
+box.execute('INSERT INTO t2 VALUES (NULL), (NULL), (NULL);')
+---
+- autoincrement_ids:
+  - 1
+  - 2
+  - 3
+  row_count: 3
+...
+box.execute('SELECT * FROM t1;')
+---
+- metadata:
+  - name: I
+    type: integer
+  rows:
+  - [100]
+  - [101]
+  - [102]
+  - [103]
+...
+box.execute('SELECT * FROM t2;')
+---
+- metadata:
+  - name: I
+    type: integer
+  rows:
+  - [1]
+  - [2]
+  - [3]
+...
+box.execute('CREATE TRIGGER r2 AFTER INSERT ON t3 FOR EACH ROW BEGIN INSERT INTO t2 VALUES (null); END')
+---
+- row_count: 1
+...
+box.execute('INSERT INTO t3 VALUES (NULL), (NULL), (NULL);')
+---
+- autoincrement_ids:
+  - 1
+  - 2
+  - 3
+  row_count: 3
+...
+box.execute('SELECT * FROM t1;')
+---
+- metadata:
+  - name: I
+    type: integer
+  rows:
+  - [100]
+  - [101]
+  - [102]
+  - [103]
+  - [104]
+  - [105]
+  - [106]
+...
+box.execute('SELECT * FROM t2;')
+---
+- metadata:
+  - name: I
+    type: integer
+  rows:
+  - [1]
+  - [2]
+  - [3]
+  - [4]
+  - [5]
+  - [6]
+...
+box.execute('SELECT * FROM t3;')
+---
+- metadata:
+  - name: I
+    type: integer
+  rows:
+  - [1]
+  - [2]
+  - [3]
+...
+box.execute('DROP TABLE t1;')
+---
+- row_count: 1
+...
+box.execute('DROP TABLE t2;')
+---
+- row_count: 1
+...
+box.execute('DROP TABLE t3;')
+---
+- row_count: 1
+...

test/sql/triggers.test.lua

@@ -191,3 +191,27 @@ box.execute([[CREATE TRIGGER r1 AFTER INSERT ON t1 FOR EACH ROW BEGIN SELECT 1;
 box.session.su('admin')
 box.schema.user.drop('tester')
 box.execute("DROP TABLE t1;")
+
+--
+-- gh-4188: Additional generated identifiers when INSERT is
+-- executed by triggers.
+--
+box.execute('CREATE TABLE t1 (i INT PRIMARY KEY AUTOINCREMENT);')
+box.execute('CREATE TABLE t2 (i INT PRIMARY KEY AUTOINCREMENT);')
+box.execute('CREATE TABLE t3 (i INT PRIMARY KEY AUTOINCREMENT);')
+
+box.execute('CREATE TRIGGER r1 AFTER INSERT ON t2 FOR EACH ROW BEGIN INSERT INTO t1 VALUES (null); END')
+box.execute('INSERT INTO t1 VALUES (100);')
+box.execute('INSERT INTO t2 VALUES (NULL), (NULL), (NULL);')
+box.execute('SELECT * FROM t1;')
+box.execute('SELECT * FROM t2;')
+
+box.execute('CREATE TRIGGER r2 AFTER INSERT ON t3 FOR EACH ROW BEGIN INSERT INTO t2 VALUES (null); END')
+box.execute('INSERT INTO t3 VALUES (NULL), (NULL), (NULL);')
+box.execute('SELECT * FROM t1;')
+box.execute('SELECT * FROM t2;')
+box.execute('SELECT * FROM t3;')
+
+box.execute('DROP TABLE t1;')
+box.execute('DROP TABLE t2;')
+box.execute('DROP TABLE t3;')