From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org Cc: korablev@tarantool.org Subject: [tarantool-patches] [PATCH 2/2] sql: transactional DDL Date: Sat, 20 Jul 2019 00:49:41 +0200 [thread overview] Message-ID: <5863bb4286a3e45b3521420273d89347bd331a92.1563576462.git.v.shpilevoy@tarantool.org> (raw) In-Reply-To: <cover.1563576462.git.v.shpilevoy@tarantool.org> Box recently added support of transactional DDL allowing to do any number of non-yielding DDL operations atomically. This is really a big relief of one of the biggest pains of SQL. Before this patch each multirow SQL DDL statement needed to prepare its own rollback procedure for a case if something would go wrong. Now with box support SQL wraps each DDL statement into a transaction, and doesn't need own escape-routes in a form of 'struct save_record' and others. Closes #4086 --- src/box/sql/build.c | 159 ++----------- src/box/sql/parse.y | 12 +- src/box/sql/prepare.c | 1 - src/box/sql/sqlInt.h | 7 +- src/box/sql/trigger.c | 6 +- src/box/sql/vdbe.c | 19 +- test/sql/ddl.result | 356 +++++++++++++++++++++++++++++ test/sql/ddl.test.lua | 187 +++++++++++++++ test/sql/errinj.result | 50 ---- test/sql/errinj.test.lua | 22 -- test/sql/view_delayed_wal.result | 12 +- test/sql/view_delayed_wal.test.lua | 8 +- 12 files changed, 598 insertions(+), 241 deletions(-) create mode 100644 test/sql/ddl.result create mode 100644 test/sql/ddl.test.lua diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 2aefa2a3f..4884a7855 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -57,64 +57,6 @@ #include "box/tuple_format.h" #include "box/coll_id_cache.h" -/** - * Structure that contains information about record that was - * inserted into system space. - */ -struct saved_record -{ - /** A link in a record list. */ - struct rlist link; - /** Id of space in which the record was inserted. */ - uint32_t space_id; - /** First register of the key of the record. */ - int reg_key; - /** Number of registers the key consists of. */ - int reg_key_count; - /** The address of the opcode. */ - int op_addr; - /** Flag to show that operation is SInsert. */ - bool is_insert; -}; - -/** - * Save inserted in system space record in list. This procedure is - * called after generation of either OP_SInsert or OP_NoColflict + - * OP_SetDiag. In the first case, record inserted to the system - * space is supposed to be deleted on error; in the latter - jump - * target specified in OP_SetDiag should be adjusted to the start - * of clean-up routines (current entry isn't inserted to the space - * yet, so there's no need to delete it). - * - * @param parser SQL Parser object. - * @param space_id Id of table in which record is inserted. - * @param reg_key Register that contains first field of the key. - * @param reg_key_count Exact number of fields of the key. - * @param op_addr Address of opcode (OP_SetDiag or OP_SInsert). - * Used to fix jump target (see - * sql_finish_coding()). - * @param is_insert_op Whether opcode is OP_SInsert or not. - */ -static inline void -save_record(struct Parse *parser, uint32_t space_id, int reg_key, - int reg_key_count, int op_addr, bool is_insert_op) -{ - struct saved_record *record = - region_alloc(&parser->region, sizeof(*record)); - if (record == NULL) { - diag_set(OutOfMemory, sizeof(*record), "region_alloc", - "record"); - parser->is_aborted = true; - return; - } - record->space_id = space_id; - record->reg_key = reg_key; - record->reg_key_count = reg_key_count; - record->op_addr = op_addr; - record->is_insert = is_insert_op; - rlist_add_entry(&parser->record_list, record, link); -} - void sql_finish_coding(struct Parse *parse_context) { @@ -122,52 +64,6 @@ sql_finish_coding(struct Parse *parse_context) struct sql *db = parse_context->db; struct Vdbe *v = sqlGetVdbe(parse_context); sqlVdbeAddOp0(v, OP_Halt); - /* - * In case statement "CREATE TABLE ..." fails it can - * left some records in system spaces that shouldn't be - * there. To clean-up properly this code is added. Last - * record isn't deleted because if statement fails than - * it won't be created. This code works the same way for - * other "CREATE ..." statements but it won't delete - * anything as these statements create no more than one - * record. Hence for processed insertions we should remove - * entries from corresponding system spaces alongside - * with fixing jump address for OP_SInsert opcode in - * case it fails during VDBE runtime; for OP_SetDiag only - * adjust jump target to the start of clean-up program - * for already inserted entries. - */ - if (!rlist_empty(&parse_context->record_list)) { - struct saved_record *record = - rlist_shift_entry(&parse_context->record_list, - struct saved_record, link); - /* - * Set jump target for OP_SetDiag and OP_SInsert. - */ - sqlVdbeChangeP2(v, record->op_addr, v->nOp); - MAYBE_UNUSED const char *comment = - "Delete entry from %s if CREATE TABLE fails"; - rlist_foreach_entry(record, &parse_context->record_list, link) { - if (record->is_insert) { - int rec_reg = ++parse_context->nMem; - sqlVdbeAddOp3(v, OP_MakeRecord, record->reg_key, - record->reg_key_count, rec_reg); - sqlVdbeAddOp2(v, OP_SDelete, record->space_id, - rec_reg); - MAYBE_UNUSED struct space *space = - space_by_id(record->space_id); - VdbeComment((v, comment, space_name(space))); - } - /* - * Set jump target for OP_SetDiag and - * OP_SInsert. - */ - sqlVdbeChangeP2(v, record->op_addr, v->nOp); - } - sqlVdbeAddOp1(v, OP_Halt, -1); - VdbeComment((v, - "Exit with an error if CREATE statement fails")); - } if (db->mallocFailed) parse_context->is_aborted = true; @@ -933,8 +829,7 @@ vdbe_emit_create_index(struct Parse *parse, struct space_def *def, sqlVdbeAddOp4(v, OP_Blob, index_parts_sz, entry_reg + 5, SQL_SUBTYPE_MSGPACK, index_parts, P4_STATIC); sqlVdbeAddOp3(v, OP_MakeRecord, entry_reg, 6, tuple_reg); - sqlVdbeAddOp3(v, OP_SInsert, BOX_INDEX_ID, 0, tuple_reg); - save_record(parse, BOX_INDEX_ID, entry_reg, 2, v->nOp - 1, true); + sqlVdbeAddOp2(v, OP_SInsert, BOX_INDEX_ID, tuple_reg); return; error: parse->is_aborted = true; @@ -991,9 +886,8 @@ vdbe_emit_space_create(struct Parse *pParse, int space_id_reg, sqlVdbeAddOp4(v, OP_Blob, table_stmt_sz, iFirstCol + 6, SQL_SUBTYPE_MSGPACK, table_stmt, P4_STATIC); sqlVdbeAddOp3(v, OP_MakeRecord, iFirstCol, 7, tuple_reg); - sqlVdbeAddOp3(v, OP_SInsert, BOX_SPACE_ID, 0, tuple_reg); + sqlVdbeAddOp2(v, OP_SInsert, BOX_SPACE_ID, tuple_reg); sqlVdbeChangeP5(v, OPFLAG_NCHANGE); - save_record(pParse, BOX_SPACE_ID, iFirstCol, 1, v->nOp - 1, true); return; error: pParse->is_aborted = true; @@ -1102,8 +996,7 @@ vdbe_emit_ck_constraint_create(struct Parse *parser, * Occupy registers for 5 fields: each member in * _ck_constraint space plus one for final msgpack tuple. */ - int ck_constraint_reg = parser->nMem + 1; - parser->nMem += 6; + int ck_constraint_reg = sqlGetTempRange(parser, 6); sqlVdbeAddOp2(v, OP_SCopy, reg_space_id, ck_constraint_reg); sqlVdbeAddOp4(v, OP_String8, 0, ck_constraint_reg + 1, 0, sqlDbStrDup(db, ck_def->name), P4_DYNAMIC); @@ -1120,13 +1013,12 @@ vdbe_emit_ck_constraint_create(struct Parse *parser, if (vdbe_emit_halt_with_presence_test(parser, BOX_CK_CONSTRAINT_ID, 0, ck_constraint_reg, 2, ER_CONSTRAINT_EXISTS, error_msg, - false, OP_NoConflict, true) != 0) + false, OP_NoConflict) != 0) return; - sqlVdbeAddOp3(v, OP_SInsert, BOX_CK_CONSTRAINT_ID, 0, + sqlVdbeAddOp2(v, OP_SInsert, BOX_CK_CONSTRAINT_ID, ck_constraint_reg + 5); - save_record(parser, BOX_CK_CONSTRAINT_ID, ck_constraint_reg, 2, - v->nOp - 1, true); VdbeComment((v, "Create CK constraint %s", ck_def->name)); + sqlReleaseTempRange(parser, ck_constraint_reg, 6); } /** @@ -1148,8 +1040,7 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context, * Occupy registers for 9 fields: each member in * _fk_constraint space plus one for final msgpack tuple. */ - int constr_tuple_reg = parse_context->nMem + 1; - parse_context->nMem += 10; + int constr_tuple_reg = sqlGetTempRange(parse_context, 10); char *name_copy = sqlDbStrDup(parse_context->db, fk->name); if (name_copy == NULL) return; @@ -1185,7 +1076,7 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context, BOX_FK_CONSTRAINT_ID, 0, constr_tuple_reg, 2, ER_CONSTRAINT_EXISTS, error_msg, - false, OP_NoConflict, true) != 0) + false, OP_NoConflict) != 0) return; sqlVdbeAddOp2(vdbe, OP_Bool, fk->is_deferred, constr_tuple_reg + 3); sqlVdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 4, 0, @@ -1228,14 +1119,13 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context, parent_links, P4_DYNAMIC); sqlVdbeAddOp3(vdbe, OP_MakeRecord, constr_tuple_reg, 9, constr_tuple_reg + 9); - sqlVdbeAddOp3(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID, 0, - constr_tuple_reg + 9); + sqlVdbeAddOp2(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID, + constr_tuple_reg + 9); if (parse_context->create_table_def.new_space == NULL) { sqlVdbeCountChanges(vdbe); sqlVdbeChangeP5(vdbe, OPFLAG_NCHANGE); } - save_record(parse_context, BOX_FK_CONSTRAINT_ID, constr_tuple_reg, 2, - vdbe->nOp - 1, true); + sqlReleaseTempRange(parse_context, constr_tuple_reg, 10); return; error: parse_context->is_aborted = true; @@ -1331,7 +1221,7 @@ sqlEndTable(struct Parse *pParse) if (vdbe_emit_halt_with_presence_test(pParse, BOX_SPACE_ID, 2, name_reg, 1, ER_SPACE_EXISTS, error_msg, (no_err != 0), - OP_NoConflict, false) != 0) + OP_NoConflict) != 0) return; int reg_space_id = getNewSpaceId(pParse); @@ -1354,18 +1244,13 @@ sqlEndTable(struct Parse *pParse) int reg_seq_record = emitNewSysSequenceRecord(pParse, reg_seq_id, new_space->def->name); - sqlVdbeAddOp3(v, OP_SInsert, BOX_SEQUENCE_ID, 0, - reg_seq_record); - save_record(pParse, BOX_SEQUENCE_ID, reg_seq_record + 1, 1, - v->nOp - 1, true); + sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, reg_seq_record); /* Do an insertion into _space_sequence. */ int reg_space_seq_record = emitNewSysSpaceSequenceRecord(pParse, reg_space_id, reg_seq_id, new_space->index[0]->def); - sqlVdbeAddOp3(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID, 0, - reg_space_seq_record); - save_record(pParse, BOX_SPACE_SEQUENCE_ID, - reg_space_seq_record + 1, 1, v->nOp - 1, true); + sqlVdbeAddOp2(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID, + reg_space_seq_record); } /* Code creation of FK constraints, if any. */ struct fk_constraint_parse *fk_parse; @@ -1492,7 +1377,7 @@ sql_create_view(struct Parse *parse_context) if (vdbe_emit_halt_with_presence_test(parse_context, BOX_SPACE_ID, 2, name_reg, 1, ER_SPACE_EXISTS, error_msg, (no_err != 0), - OP_NoConflict, false) != 0) + OP_NoConflict) != 0) goto create_view_fail; vdbe_emit_space_create(parse_context, getNewSpaceId(parse_context), @@ -1611,7 +1496,7 @@ vdbe_emit_fk_constraint_drop(struct Parse *parse_context, char *constraint_name, BOX_FK_CONSTRAINT_ID, 0, key_reg, 2, ER_NO_SUCH_CONSTRAINT, error_msg, false, - OP_Found, false) != 0) { + OP_Found) != 0) { sqlDbFree(parse_context->db, constraint_name); return; } @@ -1643,8 +1528,7 @@ vdbe_emit_ck_constraint_drop(struct Parse *parser, const char *ck_name, tt_sprintf(tnt_errcode_desc(ER_NO_SUCH_CONSTRAINT), ck_name); if (vdbe_emit_halt_with_presence_test(parser, BOX_CK_CONSTRAINT_ID, 0, key_reg, 2, ER_NO_SUCH_CONSTRAINT, - error_msg, false, - OP_Found, false) != 0) + error_msg, false, OP_Found) != 0) return; sqlVdbeAddOp3(v, OP_MakeRecord, key_reg, 2, key_reg + 2); sqlVdbeAddOp2(v, OP_SDelete, BOX_CK_CONSTRAINT_ID, key_reg + 2); @@ -3310,7 +3194,7 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id, int index_id, int key_reg, uint32_t key_len, int tarantool_error_code, const char *error_src, bool no_error, - int cond_opcode, bool is_clean_needed) + int cond_opcode) { assert(cond_opcode == OP_NoConflict || cond_opcode == OP_Found); struct Vdbe *v = sqlGetVdbe(parser); @@ -3331,10 +3215,7 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id, } else { sqlVdbeAddOp4(v, OP_SetDiag, tarantool_error_code, 0, 0, error, P4_DYNAMIC); - if (is_clean_needed) - save_record(parser, 0, 0, 0, v->nOp - 1, false); - else - sqlVdbeAddOp1(v, OP_Halt, -1); + sqlVdbeAddOp2(v, OP_Halt, -1, ON_CONFLICT_ACTION_ABORT); } sqlVdbeJumpHere(v, addr); sqlVdbeAddOp1(v, OP_Close, cursor); diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 2a60ad25b..06eab7f2d 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -172,6 +172,7 @@ cmd ::= create_table create_table_args. create_table ::= CREATE TABLE ifnotexists(E) nm(Y). { create_table_def_init(&pParse->create_table_def, &Y, E); pParse->create_table_def.new_space = sqlStartTable(pParse, &Y); + pParse->initiateTTrans = true; } %type ifnotexists {int} @@ -380,12 +381,14 @@ resolvetype(A) ::= REPLACE. {A = ON_CONFLICT_ACTION_REPLACE;} cmd ::= DROP TABLE ifexists(E) fullname(X) . { struct Token t = Token_nil; drop_table_def_init(&pParse->drop_table_def, X, &t, E); + pParse->initiateTTrans = true; sql_drop_table(pParse); } cmd ::= DROP VIEW ifexists(E) fullname(X) . { struct Token t = Token_nil; drop_view_def_init(&pParse->drop_view_def, X, &t, E); + pParse->initiateTTrans = true; sql_drop_table(pParse); } @@ -399,6 +402,7 @@ cmd ::= CREATE(X) VIEW ifnotexists(E) nm(Y) eidlist_opt(C) AS select(S). { if (!pParse->parse_only) { create_view_def_init(&pParse->create_view_def, &Y, &X, C, S, E); + pParse->initiateTTrans = true; sql_create_view(pParse); } else { sql_store_select(pParse, S); @@ -1404,6 +1408,7 @@ cmd ::= CREATE uniqueflag(U) INDEX ifnotexists(NE) nm(X) } create_index_def_init(&pParse->create_index_def, src_list, &X, Z, U, SORT_ORDER_ASC, NE); + pParse->initiateTTrans = true; sql_create_index(pParse); } @@ -1456,6 +1461,7 @@ eidlist(A) ::= nm(Y). { // cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). { drop_index_def_init(&pParse->drop_index_def, Y, &X, E); + pParse->initiateTTrans = true; sql_drop_index(pParse); } @@ -1502,7 +1508,7 @@ cmd ::= CREATE trigger_decl(A) BEGIN trigger_cmd_list(S) END(Z). { Token all; all.z = A.z; all.n = (int)(Z.z - A.z) + Z.n; - pParse->initiateTTrans = false; + pParse->initiateTTrans = true; sql_trigger_finish(pParse, S, &all); } @@ -1652,6 +1658,7 @@ raisetype(A) ::= FAIL. {A = ON_CONFLICT_ACTION_FAIL;} cmd ::= DROP TRIGGER ifexists(NOERR) fullname(X). { struct Token t = Token_nil; drop_trigger_def_init(&pParse->drop_trigger_def, X, &t, NOERR); + pParse->initiateTTrans = true; sql_drop_trigger(pParse); } @@ -1671,6 +1678,7 @@ alter_table_start(A) ::= ALTER TABLE fullname(T) . { A = T; } alter_add_constraint(A) ::= alter_table_start(T) ADD CONSTRAINT nm(N). { A.table_name = T; A.name = N; + pParse->initiateTTrans = true; } cmd ::= alter_add_constraint(N) FOREIGN KEY LP eidlist(FA) RP REFERENCES @@ -1697,11 +1705,13 @@ unique_spec(U) ::= PRIMARY KEY. { U = SQL_INDEX_TYPE_CONSTRAINT_PK; } cmd ::= alter_table_start(A) RENAME TO nm(N). { rename_entity_def_init(&pParse->rename_entity_def, A, &N); + pParse->initiateTTrans = true; sql_alter_table_rename(pParse); } cmd ::= ALTER TABLE fullname(X) DROP CONSTRAINT nm(Z). { drop_fk_def_init(&pParse->drop_fk_def, X, &Z, false); + pParse->initiateTTrans = true; sql_drop_foreign_key(pParse); } diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c index 84fb31bcd..36c21a221 100644 --- a/src/box/sql/prepare.c +++ b/src/box/sql/prepare.c @@ -243,7 +243,6 @@ sql_parser_create(struct Parse *parser, struct sql *db, uint32_t sql_flags) memset(parser, 0, sizeof(struct Parse)); parser->db = db; parser->sql_flags = sql_flags; - rlist_create(&parser->record_list); region_create(&parser->region, &cord()->slabc); } diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 4f5bad287..99296b4b3 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -2329,11 +2329,6 @@ struct Parse { * sqlEndTable() function). */ struct create_table_def create_table_def; - /** - * List of all records that were inserted in system spaces - * in current statement. - */ - struct rlist record_list; bool initiateTTrans; /* Initiate Tarantool transaction */ /** If set - do not emit byte code at all, just parse. */ bool parse_only; @@ -4542,7 +4537,7 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id, int index_id, int key_reg, uint32_t key_len, int tarantool_error_code, const char *error_src, bool no_error, - int cond_opcode, bool is_clean_needed); + int cond_opcode); /** * Generate VDBE code to delete records from system _sql_stat1 or diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 562723959..d746ef893 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -112,8 +112,7 @@ sql_trigger_begin(struct Parse *parse) name_reg, 1, ER_TRIGGER_EXISTS, error_msg, (no_err != 0), - OP_NoConflict, - false) != 0) + OP_NoConflict) != 0) goto trigger_cleanup; } @@ -421,8 +420,7 @@ sql_drop_trigger(struct Parse *parser) sqlVdbeAddOp4(v, OP_String8, 0, name_reg, 0, name_copy, P4_DYNAMIC); if (vdbe_emit_halt_with_presence_test(parser, BOX_TRIGGER_ID, 0, name_reg, 1, ER_NO_SUCH_TRIGGER, - error_msg, no_err, OP_Found, - false) != 0) + error_msg, no_err, OP_Found) != 0) goto drop_trigger_cleanup; vdbe_code_drop_trigger(parser, trigger_name, true); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 6a4a303b9..a71b331f8 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -4359,8 +4359,8 @@ case OP_Update: { break; } -/* Opcode: SInsert P1 P2 P3 * P5 - * Synopsis: space id = P1, key = r[P3], on error goto P2 +/* Opcode: SInsert P1 P2 * * P5 + * Synopsis: space id = P1, key = r[P2] * * This opcode is used only during DDL routine. * In contrast to ordinary insertion, insertion to system spaces @@ -4373,15 +4373,16 @@ case OP_Update: { */ case OP_SInsert: { assert(pOp->p1 > 0); - assert(pOp->p2 > 0); - assert(pOp->p3 >= 0); + assert(pOp->p2 >= 0); - pIn3 = &aMem[pOp->p3]; + pIn2 = &aMem[pOp->p2]; struct space *space = space_by_id(pOp->p1); assert(space != NULL); assert(space_is_system(space)); - if (tarantoolsqlInsert(space, pIn3->z, pIn3->z + pIn3->n) != 0) - goto jump_to_p2; + if (tarantoolsqlInsert(space, pIn2->z, pIn2->z + pIn2->n) != 0) { + p->errorAction = ON_CONFLICT_ACTION_ABORT; + goto abort_due_to_error; + } if (pOp->p5 & OPFLAG_NCHANGE) p->nChange++; break; @@ -4404,8 +4405,10 @@ case OP_SDelete: { struct space *space = space_by_id(pOp->p1); assert(space != NULL); assert(space_is_system(space)); - if (sql_delete_by_key(space, 0, pIn2->z, pIn2->n) != 0) + if (sql_delete_by_key(space, 0, pIn2->z, pIn2->n) != 0) { + p->errorAction = ON_CONFLICT_ACTION_ABORT; goto abort_due_to_error; + } if (pOp->p5 & OPFLAG_NCHANGE) p->nChange++; break; diff --git a/test/sql/ddl.result b/test/sql/ddl.result new file mode 100644 index 000000000..8f7f91151 --- /dev/null +++ b/test/sql/ddl.result @@ -0,0 +1,356 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... +engine = test_run:get_cfg('engine') + | --- + | ... +box.execute('pragma sql_default_engine=\''..engine..'\'') + | --- + | - row_count: 0 + | ... + +-- +-- gh-4086: SQL transactional DDL. +-- +test_run:cmd("setopt delimiter ';'") + | --- + | - true + | ... +box.begin() +box.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY);') +box.execute('CREATE TABLE t2(id INTEGER PRIMARY KEY);') +box.commit(); + | --- + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +box.space.T1 ~= nil + | --- + | - true + | ... +box.space.T1.index[0] ~= nil + | --- + | - true + | ... +box.space.T2 ~= nil + | --- + | - true + | ... +box.space.T2.index[0] ~= nil + | --- + | - true + | ... + +test_run:cmd("setopt delimiter ';'") + | --- + | - true + | ... +box.begin() +box.execute('DROP TABLE t1;') +assert(box.space.T1 == nil) +assert(box.space.T2 ~= nil) +box.execute('DROP TABLE t2;') +assert(box.space.T2 == nil) +box.commit(); + | --- + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +-- +-- Use all the possible SQL DDL statements. +-- +test_run:cmd("setopt delimiter '$'") + | --- + | - true + | ... +function monster_ddl() + local _, err1, err2, err3 + box.execute([[CREATE TABLE t1(id INTEGER PRIMARY KEY, + a INTEGER, + b INTEGER);]]) + box.execute([[CREATE TABLE t2(id INTEGER PRIMARY KEY, + a INTEGER, + b INTEGER UNIQUE, + CONSTRAINT ck1 CHECK(b < 100));]]) + + box.execute('CREATE INDEX t1a ON t1(a);') + box.execute('CREATE INDEX t2a ON t2(a);') + box.execute('DROP INDEX t2a ON t2;') + + box.execute('ALTER TABLE t1 ADD CONSTRAINT ck1 CHECK(b > 0);') + box.execute('ALTER TABLE t1 ADD CONSTRAINT ck2 CHECK(a > 0);') + box.space.T1.ck_constraint.CK1:drop() + + box.execute([[ALTER TABLE t1 ADD CONSTRAINT fk1 FOREIGN KEY + (a) REFERENCES t2(b);]]) + box.execute('ALTER TABLE t1 DROP CONSTRAINT fk1;') + +-- Try random errors inside this big batch of DDL to ensure, that +-- they do not affect normal operation. + _, err1 = pcall(box.execute, 'CREATE TABLE t1(id INTEGER PRIMARY KEY);') + + box.execute([[ALTER TABLE t1 ADD CONSTRAINT fk1 FOREIGN KEY + (a) REFERENCES t2(b);]]) + + box.execute([[CREATE TABLE trigger_catcher(id INTEGER PRIMARY + KEY AUTOINCREMENT);]]) + + box.execute([[CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW + BEGIN + INSERT INTO trigger_catcher VALUES(1); + END; ]]) + + _, err2 = pcall(box.execute, 'DROP TABLE t3;') + + box.execute([[CREATE TRIGGER t2t AFTER INSERT ON t2 FOR EACH ROW + BEGIN + INSERT INTO trigger_catcher VALUES(1); + END; ]]) + + _, err3 = pcall(box.execute, 'CREATE INDEX t1a ON t1(a, b);') + + box.execute('DROP TRIGGER t2t;') + + return 'Finished ok, errors in the middle: ', err1, err2, err3 +end$ + | --- + | ... +function monster_ddl_is_clean() + assert(box.space.T1 == nil) + assert(box.space.T2 == nil) + assert(box.space._trigger:count() == 0) + assert(box.space._fk_constraint:count() == 0) + assert(box.space._ck_constraint:count() == 0) +end$ + | --- + | ... +function monster_ddl_check() + local _, err1, err2, err3, err4, res + _, err1 = pcall(box.execute, 'INSERT INTO t2 VALUES (1, 1, 101)') + box.execute('INSERT INTO t2 VALUES (1, 1, 1)') + _, err2 = pcall(box.execute, 'INSERT INTO t2 VALUES(2, 2, 1)') + _, err3 = pcall(box.execute, 'INSERT INTO t1 VALUES(1, 20, 1)') + _, err4 = pcall(box.execute, 'INSERT INTO t1 VALUES(1, -1, 1)') + box.execute('INSERT INTO t1 VALUES (1, 1, 1)') + res = box.execute('SELECT * FROM trigger_catcher') + return 'Finished ok, errors and trigger catcher content: ', err1, err2, + err3, err4, res +end$ + | --- + | ... +function monster_ddl_clear() + box.execute('DROP TRIGGER IF EXISTS t1t;') + box.execute('DROP TABLE IF EXISTS trigger_catcher;') + pcall(box.execute, 'ALTER TABLE t1 DROP CONSTRAINT fk1;') + box.execute('DROP TABLE IF EXISTS t2') + box.execute('DROP TABLE IF EXISTS t1') + monster_ddl_is_clean() +end$ + | --- + | ... +test_run:cmd("setopt delimiter ''")$ + | --- + | - true + | ... + +-- No txn. +monster_ddl() + | --- + | - 'Finished ok, errors in the middle: ' + | - Space 'T1' already exists + | - Space 'T3' does not exist + | - Index 'T1A' already exists in space 'T1' + | ... +monster_ddl_check() + | --- + | - 'Finished ok, errors and trigger catcher content: ' + | - 'Check constraint failed ''CK1'': b < 100' + | - Duplicate key exists in unique index 'unique_unnamed_T2_2' in space 'T2' + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | - 'Check constraint failed ''CK2'': a > 0' + | - metadata: + | - name: ID + | type: integer + | rows: + | - [1] + | ... +monster_ddl_clear() + | --- + | ... + +-- Both DDL and cleanup in one txn. +res = nil + | --- + | ... +box.begin() res = {monster_ddl()} monster_ddl_clear() box.commit() + | --- + | ... +res + | --- + | - - 'Finished ok, errors in the middle: ' + | - Space 'T1' already exists + | - Space 'T3' does not exist + | - Index 'T1A' already exists in space 'T1' + | ... + +-- DDL in txn, cleanup is not. +box.begin() res = {monster_ddl()} box.commit() + | --- + | ... +res + | --- + | - - 'Finished ok, errors in the middle: ' + | - Space 'T1' already exists + | - Space 'T3' does not exist + | - Index 'T1A' already exists in space 'T1' + | ... +monster_ddl_check() + | --- + | - 'Finished ok, errors and trigger catcher content: ' + | - 'Check constraint failed ''CK1'': b < 100' + | - Duplicate key exists in unique index 'unique_unnamed_T2_2' in space 'T2' + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | - 'Check constraint failed ''CK2'': a > 0' + | - metadata: + | - name: ID + | type: integer + | rows: + | - [1] + | ... +monster_ddl_clear() + | --- + | ... + +-- DDL is not in txn, cleanup is. +monster_ddl() + | --- + | - 'Finished ok, errors in the middle: ' + | - Space 'T1' already exists + | - Space 'T3' does not exist + | - Index 'T1A' already exists in space 'T1' + | ... +monster_ddl_check() + | --- + | - 'Finished ok, errors and trigger catcher content: ' + | - 'Check constraint failed ''CK1'': b < 100' + | - Duplicate key exists in unique index 'unique_unnamed_T2_2' in space 'T2' + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | - 'Check constraint failed ''CK2'': a > 0' + | - metadata: + | - name: ID + | type: integer + | rows: + | - [1] + | ... +box.begin() monster_ddl_clear() box.commit() + | --- + | ... + +-- DDL and cleanup in separate txns. +box.begin() monster_ddl() box.commit() + | --- + | ... +monster_ddl_check() + | --- + | - 'Finished ok, errors and trigger catcher content: ' + | - 'Check constraint failed ''CK1'': b < 100' + | - Duplicate key exists in unique index 'unique_unnamed_T2_2' in space 'T2' + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | - 'Check constraint failed ''CK2'': a > 0' + | - metadata: + | - name: ID + | type: integer + | rows: + | - [1] + | ... +box.begin() monster_ddl_clear() box.commit() + | --- + | ... + +-- +-- Voluntary rollback. +-- +test_run:cmd("setopt delimiter ';'") + | --- + | - true + | ... +box.begin() +box.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY);') +assert(box.space.T1 ~= nil) +box.execute('CREATE TABLE t2(id INTEGER PRIMARY KEY);') +assert(box.space.T2 ~= nil) +box.rollback(); + | --- + | ... + +box.space.T1 == nil and box.space.T2 == nil; + | --- + | - true + | ... + +box.begin() +save1 = box.savepoint() +box.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY)') +save2 = box.savepoint() +box.execute('CREATE TABLE t2(id INTEGER PRIMARY KEY, a INTEGER)') +box.execute('CREATE INDEX t2a ON t2(a)') +save3 = box.savepoint() +assert(box.space.T1 ~= nil) +assert(box.space.T2 ~= nil) +assert(box.space.T2.index.T2A ~= nil) +box.execute('DROP TABLE t2') +assert(box.space.T2 == nil) +box.rollback_to_savepoint(save3) +assert(box.space.T2 ~= nil) +assert(box.space.T2.index.T2A ~= nil) +save3 = box.savepoint() +box.execute('DROP TABLE t2') +assert(box.space.T2 == nil) +box.rollback_to_savepoint(save2) +assert(box.space.T2 == nil) +assert(box.space.T1 ~= nil) +box.rollback_to_savepoint(save1) +box.commit(); + | --- + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +box.space.T1 == nil and box.space.T2 == nil + | --- + | - true + | ... + +-- +-- Unexpected rollback. +-- + +box.begin() res = {monster_ddl()} require('fiber').yield() + | --- + | ... +res + | --- + | - - 'Finished ok, errors in the middle: ' + | - Space 'T1' already exists + | - Space 'T3' does not exist + | - Index 'T1A' already exists in space 'T1' + | ... +box.commit() + | --- + | - error: Transaction has been aborted by a fiber yield + | ... +box.rollback() + | --- + | ... +monster_ddl_clear() + | --- + | ... diff --git a/test/sql/ddl.test.lua b/test/sql/ddl.test.lua new file mode 100644 index 000000000..477158796 --- /dev/null +++ b/test/sql/ddl.test.lua @@ -0,0 +1,187 @@ +test_run = require('test_run').new() +engine = test_run:get_cfg('engine') +box.execute('pragma sql_default_engine=\''..engine..'\'') + +-- +-- gh-4086: SQL transactional DDL. +-- +test_run:cmd("setopt delimiter ';'") +box.begin() +box.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY);') +box.execute('CREATE TABLE t2(id INTEGER PRIMARY KEY);') +box.commit(); +test_run:cmd("setopt delimiter ''"); + +box.space.T1 ~= nil +box.space.T1.index[0] ~= nil +box.space.T2 ~= nil +box.space.T2.index[0] ~= nil + +test_run:cmd("setopt delimiter ';'") +box.begin() +box.execute('DROP TABLE t1;') +assert(box.space.T1 == nil) +assert(box.space.T2 ~= nil) +box.execute('DROP TABLE t2;') +assert(box.space.T2 == nil) +box.commit(); +test_run:cmd("setopt delimiter ''"); + +-- +-- Use all the possible SQL DDL statements. +-- +test_run:cmd("setopt delimiter '$'") +function monster_ddl() + local _, err1, err2, err3 + box.execute([[CREATE TABLE t1(id INTEGER PRIMARY KEY, + a INTEGER, + b INTEGER);]]) + box.execute([[CREATE TABLE t2(id INTEGER PRIMARY KEY, + a INTEGER, + b INTEGER UNIQUE, + CONSTRAINT ck1 CHECK(b < 100));]]) + + box.execute('CREATE INDEX t1a ON t1(a);') + box.execute('CREATE INDEX t2a ON t2(a);') + box.execute('DROP INDEX t2a ON t2;') + + box.execute('ALTER TABLE t1 ADD CONSTRAINT ck1 CHECK(b > 0);') + box.execute('ALTER TABLE t1 ADD CONSTRAINT ck2 CHECK(a > 0);') + box.space.T1.ck_constraint.CK1:drop() + + box.execute([[ALTER TABLE t1 ADD CONSTRAINT fk1 FOREIGN KEY + (a) REFERENCES t2(b);]]) + box.execute('ALTER TABLE t1 DROP CONSTRAINT fk1;') + +-- Try random errors inside this big batch of DDL to ensure, that +-- they do not affect normal operation. + _, err1 = pcall(box.execute, 'CREATE TABLE t1(id INTEGER PRIMARY KEY);') + + box.execute([[ALTER TABLE t1 ADD CONSTRAINT fk1 FOREIGN KEY + (a) REFERENCES t2(b);]]) + + box.execute([[CREATE TABLE trigger_catcher(id INTEGER PRIMARY + KEY AUTOINCREMENT);]]) + + box.execute([[CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW + BEGIN + INSERT INTO trigger_catcher VALUES(1); + END; ]]) + + _, err2 = pcall(box.execute, 'DROP TABLE t3;') + + box.execute([[CREATE TRIGGER t2t AFTER INSERT ON t2 FOR EACH ROW + BEGIN + INSERT INTO trigger_catcher VALUES(1); + END; ]]) + + _, err3 = pcall(box.execute, 'CREATE INDEX t1a ON t1(a, b);') + + box.execute('DROP TRIGGER t2t;') + + return 'Finished ok, errors in the middle: ', err1, err2, err3 +end$ +function monster_ddl_is_clean() + assert(box.space.T1 == nil) + assert(box.space.T2 == nil) + assert(box.space._trigger:count() == 0) + assert(box.space._fk_constraint:count() == 0) + assert(box.space._ck_constraint:count() == 0) +end$ +function monster_ddl_check() + local _, err1, err2, err3, err4, res + _, err1 = pcall(box.execute, 'INSERT INTO t2 VALUES (1, 1, 101)') + box.execute('INSERT INTO t2 VALUES (1, 1, 1)') + _, err2 = pcall(box.execute, 'INSERT INTO t2 VALUES(2, 2, 1)') + _, err3 = pcall(box.execute, 'INSERT INTO t1 VALUES(1, 20, 1)') + _, err4 = pcall(box.execute, 'INSERT INTO t1 VALUES(1, -1, 1)') + box.execute('INSERT INTO t1 VALUES (1, 1, 1)') + res = box.execute('SELECT * FROM trigger_catcher') + return 'Finished ok, errors and trigger catcher content: ', err1, err2, + err3, err4, res +end$ +function monster_ddl_clear() + box.execute('DROP TRIGGER IF EXISTS t1t;') + box.execute('DROP TABLE IF EXISTS trigger_catcher;') + pcall(box.execute, 'ALTER TABLE t1 DROP CONSTRAINT fk1;') + box.execute('DROP TABLE IF EXISTS t2') + box.execute('DROP TABLE IF EXISTS t1') + monster_ddl_is_clean() +end$ +test_run:cmd("setopt delimiter ''")$ + +-- No txn. +monster_ddl() +monster_ddl_check() +monster_ddl_clear() + +-- Both DDL and cleanup in one txn. +res = nil +box.begin() res = {monster_ddl()} monster_ddl_clear() box.commit() +res + +-- DDL in txn, cleanup is not. +box.begin() res = {monster_ddl()} box.commit() +res +monster_ddl_check() +monster_ddl_clear() + +-- DDL is not in txn, cleanup is. +monster_ddl() +monster_ddl_check() +box.begin() monster_ddl_clear() box.commit() + +-- DDL and cleanup in separate txns. +box.begin() monster_ddl() box.commit() +monster_ddl_check() +box.begin() monster_ddl_clear() box.commit() + +-- +-- Voluntary rollback. +-- +test_run:cmd("setopt delimiter ';'") +box.begin() +box.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY);') +assert(box.space.T1 ~= nil) +box.execute('CREATE TABLE t2(id INTEGER PRIMARY KEY);') +assert(box.space.T2 ~= nil) +box.rollback(); + +box.space.T1 == nil and box.space.T2 == nil; + +box.begin() +save1 = box.savepoint() +box.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY)') +save2 = box.savepoint() +box.execute('CREATE TABLE t2(id INTEGER PRIMARY KEY, a INTEGER)') +box.execute('CREATE INDEX t2a ON t2(a)') +save3 = box.savepoint() +assert(box.space.T1 ~= nil) +assert(box.space.T2 ~= nil) +assert(box.space.T2.index.T2A ~= nil) +box.execute('DROP TABLE t2') +assert(box.space.T2 == nil) +box.rollback_to_savepoint(save3) +assert(box.space.T2 ~= nil) +assert(box.space.T2.index.T2A ~= nil) +save3 = box.savepoint() +box.execute('DROP TABLE t2') +assert(box.space.T2 == nil) +box.rollback_to_savepoint(save2) +assert(box.space.T2 == nil) +assert(box.space.T1 ~= nil) +box.rollback_to_savepoint(save1) +box.commit(); +test_run:cmd("setopt delimiter ''"); + +box.space.T1 == nil and box.space.T2 == nil + +-- +-- Unexpected rollback. +-- + +box.begin() res = {monster_ddl()} require('fiber').yield() +res +box.commit() +box.rollback() +monster_ddl_clear() diff --git a/test/sql/errinj.result b/test/sql/errinj.result index 8846e5ee8..257dbafde 100644 --- a/test/sql/errinj.result +++ b/test/sql/errinj.result @@ -388,56 +388,6 @@ box.execute("DROP TABLE t3;") --- - row_count: 1 ... --- gh-3780: space without PK raises error if --- it is used in SQL queries. --- -errinj = box.error.injection ---- -... -fiber = require('fiber') ---- -... -box.execute("CREATE TABLE t (id INT PRIMARY KEY);") ---- -- row_count: 1 -... -box.execute("INSERT INTO t VALUES (1);") ---- -- row_count: 1 -... -errinj.set("ERRINJ_WAL_DELAY", true) ---- -- ok -... --- DROP TABLE consists of several steps: firstly indexes --- are deleted, then space itself. Lets make sure that if --- first part of drop is successfully finished, but resulted --- in yield, all operations on space will be blocked due to --- absence of primary key. --- -function drop_table_yield() box.execute("DROP TABLE t;") end ---- -... -f = fiber.create(drop_table_yield) ---- -... -box.execute("SELECT * FROM t;") ---- -- error: SQL does not support spaces without primary key -... -box.execute("INSERT INTO t VALUES (2);") ---- -- error: SQL does not support spaces without primary key -... -box.execute("UPDATE t SET id = 2;") ---- -- error: SQL does not support spaces without primary key -... --- Finish drop space. -errinj.set("ERRINJ_WAL_DELAY", false) ---- -- ok -... -- -- gh-3931: Store regular identifiers in case-normal form -- diff --git a/test/sql/errinj.test.lua b/test/sql/errinj.test.lua index 48b80a443..3bc1b684d 100644 --- a/test/sql/errinj.test.lua +++ b/test/sql/errinj.test.lua @@ -118,28 +118,6 @@ box.execute("INSERT INTO t3 VALUES(1, 1, 3);") errinj.set("ERRINJ_WAL_IO", false) box.execute("DROP TABLE t3;") --- gh-3780: space without PK raises error if --- it is used in SQL queries. --- -errinj = box.error.injection -fiber = require('fiber') -box.execute("CREATE TABLE t (id INT PRIMARY KEY);") -box.execute("INSERT INTO t VALUES (1);") -errinj.set("ERRINJ_WAL_DELAY", true) --- DROP TABLE consists of several steps: firstly indexes --- are deleted, then space itself. Lets make sure that if --- first part of drop is successfully finished, but resulted --- in yield, all operations on space will be blocked due to --- absence of primary key. --- -function drop_table_yield() box.execute("DROP TABLE t;") end -f = fiber.create(drop_table_yield) -box.execute("SELECT * FROM t;") -box.execute("INSERT INTO t VALUES (2);") -box.execute("UPDATE t SET id = 2;") --- Finish drop space. -errinj.set("ERRINJ_WAL_DELAY", false) - -- -- gh-3931: Store regular identifiers in case-normal form -- diff --git a/test/sql/view_delayed_wal.result b/test/sql/view_delayed_wal.result index 519794931..3faaf07b8 100644 --- a/test/sql/view_delayed_wal.result +++ b/test/sql/view_delayed_wal.result @@ -13,8 +13,8 @@ fiber = require('fiber') ... -- View reference counters are incremented before firing -- on_commit triggers (i.e. before being written into WAL), so --- it is impossible to create view on dropped (but not written --- into WAL) space. +-- it is impossible to drop a space referenced by a created, but +-- no committed view. -- box.execute('CREATE TABLE t1(id INT PRIMARY KEY)') --- @@ -49,13 +49,13 @@ box.error.injection.set("ERRINJ_WAL_DELAY", false) fiber.sleep(0.1) --- ... -box.space.T1 +box.space.T1 ~= nil --- -- null +- true ... -box.space.V1 +box.space.V1 ~= nil --- -- null +- true ... -- -- In the same way, we have to drop all referenced spaces before diff --git a/test/sql/view_delayed_wal.test.lua b/test/sql/view_delayed_wal.test.lua index 8e73b03f3..0a10d121b 100644 --- a/test/sql/view_delayed_wal.test.lua +++ b/test/sql/view_delayed_wal.test.lua @@ -5,8 +5,8 @@ fiber = require('fiber') -- View reference counters are incremented before firing -- on_commit triggers (i.e. before being written into WAL), so --- it is impossible to create view on dropped (but not written --- into WAL) space. +-- it is impossible to drop a space referenced by a created, but +-- no committed view. -- box.execute('CREATE TABLE t1(id INT PRIMARY KEY)') function create_view() box.execute('CREATE VIEW v1 AS SELECT * FROM t1') end @@ -18,8 +18,8 @@ f2 = fiber.create(drop_index_t1) f3 = fiber.create(drop_space_t1) box.error.injection.set("ERRINJ_WAL_DELAY", false) fiber.sleep(0.1) -box.space.T1 -box.space.V1 +box.space.T1 ~= nil +box.space.V1 ~= nil -- -- In the same way, we have to drop all referenced spaces before -- 2.20.1 (Apple Git-117)
next prev parent reply other threads:[~2019-07-19 22:48 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-19 22:49 [tarantool-patches] [PATCH 0/2] SQL TXN DDL Vladislav Shpilevoy 2019-07-19 22:49 ` [tarantool-patches] [PATCH 1/2] alter: do not access space in drop ck commit trigger Vladislav Shpilevoy 2019-07-19 22:49 ` Vladislav Shpilevoy [this message] 2019-07-20 7:42 ` [tarantool-patches] Re: [PATCH 2/2] sql: transactional DDL Konstantin Osipov 2019-07-20 14:02 ` Vladislav Shpilevoy 2019-07-20 15:16 ` Konstantin Osipov 2019-07-22 13:59 ` n.pettik 2019-07-22 21:35 ` Vladislav Shpilevoy 2019-07-24 13:23 ` n.pettik 2019-07-24 20:58 ` Vladislav Shpilevoy 2019-07-25 12:04 ` n.pettik 2019-07-31 13:19 ` [tarantool-patches] Re: [PATCH 0/2] SQL TXN DDL Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5863bb4286a3e45b3521420273d89347bd331a92.1563576462.git.v.shpilevoy@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] [PATCH 2/2] sql: transactional DDL' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox