sql: do not show generated IDs if INSERT failed

When the INSERT was executed as an INSERT OR IGNORE, it was
possible to get the generated autoincrement identifiers, even if
the insert failed.
For example:
CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT,
                a INT check (a > 0));
INSERT OR IGNORE INTO t VALUES (null, 1), (null, -1), (null, 2);

should return only two identifiers.

After this patch, only in case of successful insertion, the
generated identifiers will be returned. Also, row-count shows now
number of successfull insertions in case of INSERT OR IGNORE.

Follow-up #4188

src/box/sql/insert.c

@@ -737,7 +737,9 @@ sqlInsert(Parse * pParse,	/* Parser context */
 		fk_constraint_emit_check(pParse, space, 0, regIns, 0);
 		vdbe_emit_insertion_completion(v, space, regIns + 1,
 					       space->def->field_count,
-					       on_error);
+					       on_error,
+					       autoinc_fieldno < UINT32_MAX &&
+					       pParse->triggered_space == NULL);
 		/*
 		 * Save the newly generated autoincrement value to
 		 * VDBE context. We don't need to do so for
@@ -984,10 +986,13 @@ process_index:  ;
 void
 vdbe_emit_insertion_completion(struct Vdbe *v, struct space *space,
 			       int raw_data_reg, uint32_t tuple_len,
-			       enum on_conflict_action on_conflict)
+			       enum on_conflict_action on_conflict,
+			       bool is_autoinc_present)
 {
 	assert(v != NULL);
 	u16 pik_flags = OPFLAG_NCHANGE;
+	if (is_autoinc_present)
+		pik_flags |= OPFLAG_AUTOINC;
 	SET_CONFLICT_FLAG(pik_flags, on_conflict);
 	sqlVdbeAddOp3(v, OP_MakeRecord, raw_data_reg, tuple_len,
 			  raw_data_reg + tuple_len);

src/box/sql/sqlInt.h

@@ -2365,6 +2365,11 @@ struct Parse {
 #define OPFLAG_NCHANGE       0x01	/* OP_Insert: Set to update db->nChange */
 				     /* Also used in P2 (not P5) of OP_Delete */
 #define OPFLAG_EPHEM         0x01	/* OP_Column: Ephemeral output is ok */
+/*
+ * OP_IdxInsert, OP_IdxReplace: Inserted tuple can contain NULL in
+ * PK field, that will be replaced by integer using sequence.
+ */
+#define OPFLAG_AUTOINC      0x100
 #define OPFLAG_OE_IGNORE    0x200	/* OP_IdxInsert: Ignore flag */
 #define OPFLAG_OE_FAIL      0x400	/* OP_IdxInsert: Fail flag */
 #define OPFLAG_OE_ROLLBACK  0x800	/* OP_IdxInsert: Rollback flag. */
@@ -3502,11 +3507,14 @@ vdbe_emit_ck_constraint(struct Parse *parser, struct Expr *expr,
  * @param raw_data_reg Register with raw data to insert.
  * @param tuple_len Number of registers to hold the tuple.
  * @param on_conflict On conflict action.
+ * @param is_autoinc_present True if space has a field with
+ *        autoincrement.
  */
 void
 vdbe_emit_insertion_completion(struct Vdbe *v, struct space *space,
 			       int raw_data_reg, uint32_t tuple_len,
-			       enum on_conflict_action on_conflict);
+			       enum on_conflict_action on_conflict,
+			       bool is_autoinc_present);
 
 void
 sql_set_multi_write(Parse *, bool);

src/box/sql/update.c

@@ -450,7 +450,7 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 		if (on_error == ON_CONFLICT_ACTION_REPLACE) {
 			 vdbe_emit_insertion_completion(v, space, regNew,
 							space->def->field_count,
-							on_error);
+							on_error, false);
 
 		} else {
 			int key_reg;

src/box/sql/vdbe.c

@@ -4204,8 +4204,6 @@ case OP_IdxReplace:
 case OP_IdxInsert: {
 	pIn2 = &aMem[pOp->p1];
 	assert((pIn2->flags & MEM_Blob) != 0);
-	if (pOp->p5 & OPFLAG_NCHANGE)
-		p->nChange++;
 	if (ExpandBlob(pIn2) != 0)
 		goto abort_due_to_error;
 	struct space *space;
@@ -4228,8 +4226,18 @@ case OP_IdxInsert: {
 		rc = tarantoolsqlEphemeralInsert(space, pIn2->z,
 						     pIn2->z + pIn2->n);
 	}
+	if (rc == 0 && (pOp->p5 & OPFLAG_NCHANGE) != 0)
+		p->nChange++;
 
-	if (pOp->p5 & OPFLAG_OE_IGNORE) {
+	if ((pOp->p5 & OPFLAG_OE_IGNORE) != 0) {
+		/*
+		 * If we ignore errors, we must skip the
+		 * OP_SaveAutoincValue opcode, if present. This
+		 * should be the following opcode if the space has
+		 * a field with AUTOINCREMENT.
+		 */
+		if (rc != 0 && (pOp->p5 & OPFLAG_AUTOINC) != 0)
+			++pOp;
 		/* Ignore any kind of failes and do not raise error message */
 		rc = 0;
 		/* If we are in trigger, increment ignore raised counter */

test/sql/row-count.result

@@ -335,3 +335,33 @@ box.execute("DROP TABLE t1;")
 ---
 - row_count: 1
 ...
+--
+-- gh-4188: check that generated IDs is not showed for failed
+-- insertions in case of INSERT OR IGNORE.
+--
+box.execute("CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT, a INT check (a > 0));")
+---
+- row_count: 1
+...
+box.execute("INSERT OR IGNORE INTO t VALUES (null, 1), (null, -1), (null, 2);")
+---
+- autoincrement_ids:
+  - 1
+  - 3
+  row_count: 2
+...
+box.execute("SELECT * FROM t;")
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: A
+    type: integer
+  rows:
+  - [1, 1]
+  - [3, 2]
+...
+box.execute("DROP TABLE t;")
+---
+- row_count: 1
+...

test/sql/row-count.test.lua

@@ -73,3 +73,11 @@ box.execute("DROP TABLE t2;")
 box.execute("DROP TABLE t3;")
 box.execute("DROP TABLE t1;")
 
+--
+-- gh-4188: check that generated IDs is not showed for failed
+-- insertions in case of INSERT OR IGNORE.
+--
+box.execute("CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT, a INT check (a > 0));")
+box.execute("INSERT OR IGNORE INTO t VALUES (null, 1), (null, -1), (null, 2);")
+box.execute("SELECT * FROM t;")
+box.execute("DROP TABLE t;")