[tarantool-patches] [PATCH v3 4/4] sql: do not show generated IDs if INSERT failed
imeevma at tarantool.org
imeevma at tarantool.org
Thu Jul 11 11:22:38 MSK 2019
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 | 9 +++++++--
src/box/sql/sqlInt.h | 10 +++++++++-
src/box/sql/update.c | 2 +-
src/box/sql/vdbe.c | 14 +++++++++++---
test/sql/row-count.result | 26 ++++++++++++++++++++++++++
test/sql/row-count.test.lua | 7 +++++++
6 files changed, 61 insertions(+), 7 deletions(-)
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index db95a02..d162242 100644
--- a/src/box/sql/insert.c
+++ b/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);
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 73dc6e4..f668236 100644
--- a/src/box/sql/sqlInt.h
+++ b/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);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index d77bee0..d249a55 100644
--- a/src/box/sql/update.c
+++ b/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;
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 47ef0ab..884c660 100644
--- a/src/box/sql/vdbe.c
+++ b/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 */
diff --git a/test/sql/row-count.result b/test/sql/row-count.result
index e7841ca..522d2f1 100644
--- a/test/sql/row-count.result
+++ b/test/sql/row-count.result
@@ -335,3 +335,29 @@ 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]
+...
diff --git a/test/sql/row-count.test.lua b/test/sql/row-count.test.lua
index 9f5215c..30de8b4 100644
--- a/test/sql/row-count.test.lua
+++ b/test/sql/row-count.test.lua
@@ -73,3 +73,10 @@ 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;")
--
2.7.4
More information about the Tarantool-patches
mailing list