Skip to content

Commit

Permalink
sql: do not show generated IDs if INSERT failed
Browse files Browse the repository at this point in the history
When the INSERT was executed as an INSERT OR IGNORE, it was
possible to get the generated autoincrement identifiers, even if
the insert failed. After this patch, only in case of successful
insertion, the generated identifiers will be returned.

Follow-up #4188
  • Loading branch information
ImeevMA committed Jul 11, 2019
1 parent f2fb0a7 commit 65ae156
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 6 deletions.
9 changes: 7 additions & 2 deletions src/box/sql/insert.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 9 additions & 1 deletion src/box/sql/sqlInt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/box/sql/update.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 10 additions & 2 deletions src/box/sql/vdbe.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 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 ((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 */
Expand Down

0 comments on commit 65ae156

Please sign in to comment.