sql: do not show additional generated ids

Currently, if INSERT is executed by a trigger, the new generated
identifiers will be stored in the VDBE. This is wrong, and this
patch fixes it. Only identifiers generated during INSERT run by
the user will be saved.

Closes #4188

src/box/errcode.h

@@ -249,6 +249,7 @@ struct errcode_record {
 	/*194 */_(ER_MULTIKEY_INDEX_MISMATCH,	"Field %s is used as multikey in one index and as single key in another") \
 	/*195 */_(ER_CREATE_CK_CONSTRAINT,	"Failed to create check constraint '%s': %s") \
 	/*196 */_(ER_CK_CONSTRAINT_FAILED,	"Check constraint failed '%s': %s") \
+	/*197 */_(ER_SEQUENCE_NO_ELEMENTS,	"Sequence '%s' has no elements") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file

src/box/sequence.c

@@ -195,9 +195,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);
@@ -232,9 +229,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) {
@@ -347,3 +341,19 @@ 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);
+	if (pos == light_sequence_end) {
+		diag_set(ClientError, ER_SEQUENCE_NO_ELEMENTS, seq->def->name);
+		return -1;
+	}
+	struct sequence_data data = light_sequence_get(&sequence_data_index,
+						       pos);
+	*out_value = data.value;
+	return 0;
+}

src/box/sequence.h

@@ -140,9 +140,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.
  *
@@ -153,6 +150,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" */
 #endif /* defined(__cplusplus) */

src/box/sql/insert.c

@@ -247,7 +247,8 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	      SrcList * pTabList,	/* Name of table into which we are inserting */
 	      Select * pSelect,	/* A SELECT statement to use as the data source */
 	      IdList * pColumn,	/* Column names corresponding to IDLIST. */
-	      enum on_conflict_action on_error)
+	      enum on_conflict_action on_error,
+	      bool is_triggered)
 {
 	sql *db;		/* The main database structure */
 	char *zTab;		/* Name of the table into which we are inserting */
@@ -644,10 +645,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 			if (j < 0 || nColumn == 0
 			    || (pColumn && j >= pColumn->nId)) {
 				if (i == (int) autoinc_fieldno) {
-					sqlVdbeAddOp2(v,
-							  OP_NextAutoincValue,
-							  space->def->id,
-							  iRegStore);
+					sqlVdbeAddOp2(v, OP_Null, 0, iRegStore);
 					continue;
 				}
 				struct Expr *dflt = NULL;
@@ -746,7 +744,6 @@ sqlInsert(Parse * pParse,	/* Parser context */
 						iRegStore);
 			}
 		}
-
 		/*
 		 * Generate code to check constraints and process
 		 * final insertion.
@@ -757,6 +754,13 @@ sqlInsert(Parse * pParse,	/* Parser context */
 		vdbe_emit_insertion_completion(v, space, regIns + 1,
 					       space->def->field_count,
 					       on_error);
+		/*
+		 * Save the newly autogenerated value to VDBE.
+		 */
+		if (!is_triggered && autoinc_fieldno < UINT32_MAX) {
+			sqlVdbeAddOp2(v, OP_NextAutoincValue, space->def->id,
+				      regData + autoinc_fieldno);
+		}
 	}
 
 	/* Update the count of rows that are inserted

src/box/sql/parse.y

@@ -839,15 +839,15 @@ cmd ::= with(W) insert_cmd(R) INTO fullname(X) idlist_opt(F) select(S). {
   sqlSubProgramsRemaining = SQL_MAX_COMPILING_TRIGGERS;
   /* Instruct SQL to initate Tarantool's transaction.  */
   pParse->initiateTTrans = true;
-  sqlInsert(pParse, X, S, F, R);
+  sqlInsert(pParse, X, S, F, R, false);
 }
 cmd ::= with(W) insert_cmd(R) INTO fullname(X) idlist_opt(F) DEFAULT VALUES.
 {
   sqlWithPush(pParse, W, 1);
   sqlSubProgramsRemaining = SQL_MAX_COMPILING_TRIGGERS;
   /* Instruct SQL to initate Tarantool's transaction.  */
   pParse->initiateTTrans = true;
-  sqlInsert(pParse, X, 0, F, R);
+  sqlInsert(pParse, X, 0, F, R, false);
 }
 
 %type insert_cmd {int}

src/box/sql/sqlInt.h

@@ -3446,7 +3446,7 @@ sql_store_select(struct Parse *parse_context, struct Select *select);
 void
 sql_drop_table(struct Parse *);
 void sqlInsert(Parse *, SrcList *, Select *, IdList *,
-	       enum on_conflict_action);
+	       enum on_conflict_action, bool);
 void *sqlArrayAllocate(sql *, void *, int, int *, int *);
 
 /**

src/box/sql/trigger.c

@@ -631,14 +631,10 @@ codeTriggerProgram(Parse * pParse,	/* The parser context */
 				break;
 			}
 		case TK_INSERT:{
-				sqlInsert(pParse,
-					      targetSrcList(pParse, pStep),
-					      sqlSelectDup(db,
-							       pStep->pSelect,
-							       0),
-					      sqlIdListDup(db,
-							       pStep->pIdList),
-					      pParse->eOrconf);
+				sqlInsert(pParse, targetSrcList(pParse, pStep),
+					  sqlSelectDup(db, pStep->pSelect, 0),
+					  sqlIdListDup(db, pStep->pIdList),
+					  pParse->eOrconf, true);
 				break;
 			}
 		case TK_DELETE:{

src/box/sql/vdbe.c

@@ -574,7 +574,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);
@@ -1268,35 +1268,30 @@ case OP_String: {          /* out2 */
 	break;
 }
 
-/* Opcode: NextAutoincValue P1 P2 * * *
- * Synopsis: r[P2] = next value from space sequence, which pageno is r[P1]
+/* Opcode: NextAutoincValue P1 * * * *
+ * Synopsis: save last value from sequence of space with id 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.
+ * Get last value from space sequence, which space id is written
+ * into register P1, save this value into VDBE.
  */
 case OP_NextAutoincValue: {
 	assert(pOp->p1 > 0);
 	assert(pOp->p2 > 0);
 
-	struct space *space = space_by_id(pOp->p1);
-	if (space == NULL) {
-		rc = SQL_TARANTOOL_ERROR;
-		goto abort_due_to_error;
-	}
+	pIn2 = &p->aMem[pOp->p2];
+	if ((pIn2->flags & MEM_Null) == 0)
+		break;
 
+	struct space *space = space_by_id(pOp->p1);
+	assert(space != NULL);
+	assert(space->sequence != NULL);
 	int64_t value;
-	struct sequence *sequence = space->sequence;
-	if (sequence == NULL || sequence_next(sequence, &value) != 0) {
+	if (sequence_get_value(space->sequence, &value) != 0) {
 		rc = SQL_TARANTOOL_ERROR;
 		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;
 }
 
@@ -3112,7 +3107,7 @@ case OP_CheckViewReferences: {
  * Otherwise, raise an error with appropriate error message.
  */
 case OP_TransactionBegin: {
-	if (sql_txn_begin(p) != 0) {
+	if (sql_txn_begin() != 0) {
 		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
@@ -3175,7 +3170,7 @@ case OP_TransactionRollback: {
  */
 case OP_TTransaction: {
 	if (!box_txn()) {
-		if (sql_txn_begin(p) != 0) {
+		if (sql_txn_begin() != 0) {
 			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}

src/box/sql/vdbe.h

@@ -179,11 +179,10 @@ Vdbe *sqlVdbeCreate(Parse *);
  * Allocate and initialize SQL-specific struct which completes
  * original Tarantool's txn struct using region allocator.
  *
- * @param v Vdbe with which associate this transaction.
  * @retval NULL on OOM, new psql_txn struct on success.
  **/
 struct sql_txn *
-sql_alloc_txn(struct Vdbe *v);
+sql_alloc_txn();
 
 /**
  * Prepare given VDBE to execution: initialize structs connected

src/box/sql/vdbeInt.h

@@ -481,7 +481,7 @@ void sqlVdbeDeleteAuxData(sql *, AuxData **, int, int);
 int sqlVdbeExec(Vdbe *);
 int sqlVdbeList(Vdbe *);
 int
-sql_txn_begin(Vdbe *p);
+sql_txn_begin();
 Savepoint *
 sql_savepoint(Vdbe *p,
 	      const char *zName);

src/box/sql/vdbeaux.c

@@ -77,7 +77,7 @@ sqlVdbeCreate(Parse * pParse)
 }
 
 struct sql_txn *
-sql_alloc_txn(struct Vdbe *v)
+sql_alloc_txn()
 {
 	struct sql_txn *txn = region_alloc_object(&fiber()->gc,
 						  struct sql_txn);
@@ -86,7 +86,6 @@ sql_alloc_txn(struct Vdbe *v)
 			 "struct sql_txn");
 		return NULL;
 	}
-	txn->vdbe = v;
 	txn->pSavepoint = NULL;
 	txn->fk_deferred_count = 0;
 	return txn;
@@ -106,11 +105,10 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
 		 * check FK violations, at least now.
 		 */
 		if (txn->psql_txn == NULL) {
-			txn->psql_txn = sql_alloc_txn(vdbe);
+			txn->psql_txn = sql_alloc_txn();
 			if (txn->psql_txn == NULL)
 				return -1;
 		}
-		txn->psql_txn->vdbe = vdbe;
 	}
 	return 0;
 }
@@ -2264,7 +2262,7 @@ sqlVdbeCheckFk(Vdbe * p, int deferred)
 }
 
 int
-sql_txn_begin(Vdbe *p)
+sql_txn_begin()
 {
 	if (in_txn()) {
 		diag_set(ClientError, ER_ACTIVE_TRANSACTION);
@@ -2273,7 +2271,7 @@ sql_txn_begin(Vdbe *p)
 	struct txn *ptxn = txn_begin(false);
 	if (ptxn == NULL)
 		return -1;
-	ptxn->psql_txn = sql_alloc_txn(p);
+	ptxn->psql_txn = sql_alloc_txn();
 	if (ptxn->psql_txn == NULL) {
 		box_txn_rollback();
 		return -1;

src/box/txn.h

@@ -118,8 +118,6 @@ struct sql_txn {
 	 * VDBE to the next in the same transaction.
 	 */
 	uint32_t fk_deferred_count;
-	/** Current VDBE. */
-	struct Vdbe *vdbe;
 };
 
 /**
@@ -378,21 +376,6 @@ txn_last_stmt(struct txn *txn)
 void
 txn_on_stop(struct trigger *trigger, void *event);
 
-/**
- * Return VDBE that is being currently executed.
- *
- * @retval VDBE that is being currently executed.
- * @retval NULL Either txn or ptxn_sql or vdbe is NULL;
- */
-static inline struct Vdbe *
-txn_vdbe()
-{
-	struct txn *txn = in_txn();
-	if (txn == NULL || txn->psql_txn == NULL)
-		return NULL;
-	return txn->psql_txn->vdbe;
-}
-
 /**
  * FFI bindings: do not throw exceptions, do not accept extra
  * arguments

test/box/misc.result

@@ -525,6 +525,7 @@ t;
   194: box.error.MULTIKEY_INDEX_MISMATCH
   195: box.error.CREATE_CK_CONSTRAINT
   196: box.error.CK_CONSTRAINT_FAILED
+  197: box.error.SEQUENCE_NO_ELEMENTS
 ...
 test_run:cmd("setopt delimiter ''");
 ---

test/sql/gh-2981-check-autoinc.result

@@ -23,6 +23,14 @@ box.execute("CREATE TABLE t3 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER,
 ---
 - row_count: 1
 ...
+box.execute("CREATE TABLE t4 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
+---
+- row_count: 1
+...
+box.execute("CREATE TABLE t5 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));");
+---
+- row_count: 1
+...
 box.execute("insert into t1 values (18, null);")
 ---
 - row_count: 1
@@ -59,6 +67,24 @@ box.execute("insert into t3(s2) values (null)")
 - error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_1_T3'':
     s1 < 10'
 ...
+box.execute("INSERT INTO t4 VALUES (18, NULL);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t4 VALUES (NULL, NULL);")
+---
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_1_T4'':
+    s1 <> 19'
+...
+box.execute("INSERT INTO t5 VALUES (18, NULL);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t5 SELECT NULL, NULL FROM t5;")
+---
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_1_T5'':
+    s1 <> 19'
+...
 box.execute("DROP TABLE t1")
 ---
 - row_count: 1