From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id EFF8242F042 for ; Mon, 14 Oct 2019 19:03:29 +0300 (MSK) From: Kirill Shcherbatov Date: Mon, 14 Oct 2019 19:03:21 +0300 Message-Id: <614b4c333ba04c42a6d5b00cb4089636ad01aed4.1571068485.git.kshcherbatov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v1 6/9] sql: rework CREATE TABLE rule in parser List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org, kostja.osipov@gmail.com, korablev@tarantool.org SQL grammar used to construct the sql_trigger object or to generate a related VDBE instructions in a two functions with a shared context: sql_trigger_begin and sql_trigger_end. It used to be an AST compile or VDBE code generation in according to parser::is_parse_only flag state - that is messy. This patch reworks SQL rule a bit to call a function once, when the whole trigger context is assembled to call the first function that constructs AST or the second to emit VDBE code. Needed for #4343. --- src/box/sql/build.c | 82 ++++++++++++++++++++++++++ src/box/sql/parse.y | 26 ++++----- src/box/sql/parse_def.h | 13 ++++- src/box/sql/sqlInt.h | 18 ++---- src/box/sql/trigger.c | 121 +++------------------------------------ test/sql/triggers.result | 16 +++--- test/sql/upgrade.result | 4 +- 7 files changed, 128 insertions(+), 152 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 9f9f80b70..5040d04d1 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1281,6 +1281,88 @@ sqlEndTable(struct Parse *pParse) } } +void +sql_create_trigger(struct Parse *parse_context) +{ + struct create_trigger_def *trigger_def = + &parse_context->create_trigger_def; + struct create_entity_def *create_def = &trigger_def->base; + struct alter_entity_def *alter_def = &create_def->base; + assert(alter_def->entity_type == ENTITY_TYPE_TRIGGER); + assert(alter_def->alter_action == ALTER_ACTION_CREATE); + struct sql *db = parse_context->db; + + char *trigger_name = sql_name_from_token(db, &create_def->name); + if (trigger_name == NULL || + sqlCheckIdentifierName(parse_context, trigger_name) != 0) { + parse_context->is_aborted = true; + goto end; + } + + struct Vdbe *v = sqlGetVdbe(parse_context); + if (v == NULL) + goto end; + sqlVdbeCountChanges(v); + const char *error_msg = tt_sprintf(tnt_errcode_desc(ER_TRIGGER_EXISTS), + trigger_name); + int name_reg = ++parse_context->nMem; + sqlVdbeAddOp4(v, OP_String8, 0, name_reg, 0, + sqlDbStrDup(db, trigger_name), P4_DYNAMIC); + if (vdbe_emit_halt_with_presence_test(parse_context, BOX_TRIGGER_ID, 0, + name_reg, 1, ER_TRIGGER_EXISTS, + error_msg, + create_def->if_not_exist, + OP_NoConflict) != 0) { + parse_context->is_aborted = true; + goto end; + } + + const char *table_name = alter_def->entity_name->a[0].zName; + uint32_t space_id = box_space_id_by_name(table_name, + strlen(table_name)); + if (space_id == BOX_ID_NIL) { + diag_set(ClientError, ER_NO_SUCH_SPACE, table_name); + parse_context->is_aborted = true; + goto end; + } + + struct space *_trigger = space_by_id(BOX_TRIGGER_ID); + assert(_trigger != NULL); + + int first_col = parse_context->nMem + 1; + parse_context->nMem += 3; + int record = ++parse_context->nMem; + + uint32_t opts_buff_sz = mp_sizeof_map(1) + + mp_sizeof_str(strlen("sql")) + + mp_sizeof_str(trigger_def->sql_len); + char *opts_buff = (char *) sqlDbMallocRaw(db, opts_buff_sz); + if (opts_buff == NULL) { + parse_context->is_aborted = true; + goto end; + } + + char *data = mp_encode_map(opts_buff, 1); + data = mp_encode_str(data, "sql", strlen("sql")); + data = mp_encode_str(data, trigger_def->sql, trigger_def->sql_len); + sqlVdbeAddOp4(v, OP_String8, 0, first_col, 0, trigger_name, P4_DYNAMIC); + sqlVdbeAddOp2(v, OP_Integer, space_id, first_col + 1); + sqlVdbeAddOp4(v, OP_Blob, opts_buff_sz, first_col + 2, + SQL_SUBTYPE_MSGPACK, opts_buff, P4_DYNAMIC); + sqlVdbeAddOp3(v, OP_MakeRecord, first_col, 3, record); + sqlVdbeAddOp4(v, OP_IdxInsert, record, 0, 0, (char *)_trigger, + P4_SPACEPTR); + sqlVdbeChangeP5(v, OPFLAG_NCHANGE); + trigger_name = NULL; + +end: + sqlDbFree(db, trigger_name); + sqlSrcListDelete(db, alter_def->entity_name); + sqlIdListDelete(db, trigger_def->cols); + sql_expr_delete(db, trigger_def->when, false); + sqlDeleteTriggerStep(db, trigger_def->step_list); +} + void sql_create_view(struct Parse *parse_context) { diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index ed59a875a..264b24f99 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -1574,21 +1574,19 @@ plus_num(A) ::= number(A). minus_num(A) ::= MINUS number(X). {A = X;} //////////////////////////// The CREATE TRIGGER command ///////////////////// -cmd ::= createkw 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 = true; - sql_trigger_finish(pParse, S, &all); -} - -trigger_decl(A) ::= TRIGGER ifnotexists(NOERR) nm(B) - trigger_time(C) trigger_event(D) - ON fullname(E) foreach_clause when_clause(G). { +cmd ::= createkw(Y) TRIGGER ifnotexists(NOERR) nm(B) trigger_time(C) + trigger_event(D) ON fullname(E) foreach_clause when_clause(G) + BEGIN trigger_cmd_list(S) END(Z). { + int sql_len = (int)(Z.z - Y.z) + Z.n; + const char *sql = Y.z; create_trigger_def_init(&pParse->create_trigger_def, E, &B, C, D.a, D.b, G, - NOERR); - sql_trigger_begin(pParse); - A = B; /*A-overwrites-T*/ + NOERR, sql, sql_len, S); + if (!pParse->parse_only) { + pParse->initiateTTrans = true; + sql_create_trigger(pParse); + } else { + sql_store_trigger(pParse); + } } %type trigger_time {int} diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h index 2502fc46c..71fa89959 100644 --- a/src/box/sql/parse_def.h +++ b/src/box/sql/parse_def.h @@ -278,6 +278,12 @@ struct create_trigger_def { struct IdList *cols; /** When clause. */ struct Expr *when; + /** Source SQL Expression. */ + const char *sql; + /** Source SQL Expression length. */ + uint32_t sql_len; + /** Steps describing SQL trigger program. */ + struct TriggerStep *step_list; }; struct create_constraint_def { @@ -412,7 +418,9 @@ static inline void create_trigger_def_init(struct create_trigger_def *trigger_def, struct SrcList *table_name, struct Token *name, int tr_tm, int op, struct IdList *cols, - struct Expr *when, bool if_not_exists) + struct Expr *when, bool if_not_exists, + const char *sql, uint32_t sql_len, + struct TriggerStep *step_list) { create_entity_def_init(&trigger_def->base, ENTITY_TYPE_TRIGGER, table_name, name, if_not_exists); @@ -420,6 +428,9 @@ create_trigger_def_init(struct create_trigger_def *trigger_def, trigger_def->event_manipulation = trigger_event_manipulation_by_op(op); trigger_def->cols = cols; trigger_def->when = when; + trigger_def->sql = sql; + trigger_def->sql_len = sql_len; + trigger_def->step_list = step_list; } static inline void diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 8c9a69a00..180997ce4 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3364,25 +3364,17 @@ sql_materialize_view(struct Parse *parse, const char *name, struct Expr *where, * statement up to the point of the BEGIN before the trigger * actions. A sql_trigger structure is generated based on the * information available and stored in parse->parsed_ast.trigger. - * After the trigger actions have been parsed, the - * sql_trigger_finish() function is called to complete the trigger - * construction process. */ void -sql_trigger_begin(struct Parse *parse); +sql_store_trigger(struct Parse *parse); /** - * This routine is called after all of the trigger actions have - * been parsed in order to complete the process of building the - * trigger. - * - * @param parse Parser context. - * @param step_list The triggered program. - * @param token Token that describes the complete CREATE TRIGGER. + * This function is called from parser to generate create trigger + * VDBE code. + * @param parser Parser context. */ void -sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list, - struct Token *token); +sql_create_trigger(struct Parse *parse); /** * This function is called from parser to generate drop trigger diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 269c09a3d..3a0ee6b7d 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -63,7 +63,7 @@ sqlDeleteTriggerStep(sql * db, TriggerStep * pTriggerStep) } void -sql_trigger_begin(struct Parse *parse) +sql_store_trigger(struct Parse *parse) { /* The new trigger. */ struct sql_trigger *trigger = NULL; @@ -95,28 +95,6 @@ sql_trigger_begin(struct Parse *parse) goto set_tarantool_error_and_cleanup; } - if (!parse->parse_only) { - struct Vdbe *v = sqlGetVdbe(parse); - if (v != NULL) - sqlVdbeCountChanges(v); - const char *error_msg = - tt_sprintf(tnt_errcode_desc(ER_TRIGGER_EXISTS), - trigger_name); - char *name_copy = sqlDbStrDup(db, trigger_name); - if (name_copy == NULL) - goto trigger_cleanup; - int name_reg = ++parse->nMem; - sqlVdbeAddOp4(parse->pVdbe, OP_String8, 0, name_reg, 0, - name_copy, P4_DYNAMIC); - bool no_err = create_def->if_not_exist; - if (vdbe_emit_halt_with_presence_test(parse, BOX_TRIGGER_ID, 0, - name_reg, 1, - ER_TRIGGER_EXISTS, - error_msg, (no_err != 0), - OP_NoConflict) != 0) - goto trigger_cleanup; - } - /* Build the Trigger object. */ trigger = (struct sql_trigger *)sqlDbMallocZero(db, sizeof(struct @@ -131,9 +109,12 @@ sql_trigger_begin(struct Parse *parse) trigger->pWhen = sqlExprDup(db, trigger_def->when, EXPRDUP_REDUCE); trigger->pColumns = sqlIdListDup(db, trigger_def->cols); rlist_create(&trigger->link); - if ((trigger->pWhen != NULL && trigger->pWhen == NULL) || - (trigger->pColumns != NULL && trigger->pColumns == NULL)) + if ((trigger_def->when != NULL && trigger->pWhen == NULL) || + (trigger_def->cols != NULL && trigger->pColumns == NULL)) goto trigger_cleanup; + trigger->step_list = parse->create_trigger_def.step_list; + parse->create_trigger_def.step_list = NULL; + assert(parse->parsed_ast.trigger == NULL); parse->parsed_ast.trigger = trigger; parse->parsed_ast_type = AST_TYPE_TRIGGER; @@ -143,6 +124,7 @@ sql_trigger_begin(struct Parse *parse) sqlSrcListDelete(db, alter_def->entity_name); sqlIdListDelete(db, trigger_def->cols); sql_expr_delete(db, trigger_def->when, false); + sqlDeleteTriggerStep(db, parse->create_trigger_def.step_list); if (parse->parsed_ast.trigger == NULL) sql_trigger_delete(db, trigger); else @@ -155,95 +137,6 @@ set_tarantool_error_and_cleanup: goto trigger_cleanup; } -void -sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list, - struct Token *token) -{ - /* Trigger being finished. */ - struct sql_trigger *trigger = parse->parsed_ast.trigger; - /* The database. */ - struct sql *db = parse->db; - - parse->parsed_ast.trigger = NULL; - if (NEVER(parse->is_aborted) || trigger == NULL) - goto cleanup; - char *trigger_name = trigger->zName; - trigger->step_list = step_list; - while (step_list != NULL) { - step_list = step_list->pNext; - } - - /* Trigger name for error reporting. */ - struct Token trigger_name_token; - sqlTokenInit(&trigger_name_token, trigger->zName); - - /* - * Generate byte code to insert a new trigger into - * Tarantool for non-parsing mode or export trigger. - */ - if (!parse->parse_only) { - /* Make an entry in the _trigger space. */ - struct Vdbe *v = sqlGetVdbe(parse); - if (v == 0) - goto cleanup; - - char *sql_str = - sqlMPrintf(db, "CREATE TRIGGER %s", token->z); - if (db->mallocFailed) - goto cleanup; - - struct space *_trigger = space_by_id(BOX_TRIGGER_ID); - assert(_trigger != NULL); - - int first_col = parse->nMem + 1; - parse->nMem += 3; - int record = ++parse->nMem; - int sql_str_len = strlen(sql_str); - int sql_len = strlen("sql"); - - uint32_t opts_buff_sz = mp_sizeof_map(1) + - mp_sizeof_str(sql_len) + - mp_sizeof_str(sql_str_len); - char *opts_buff = (char *) sqlDbMallocRaw(db, opts_buff_sz); - if (opts_buff == NULL) - goto cleanup; - - char *data = mp_encode_map(opts_buff, 1); - data = mp_encode_str(data, "sql", sql_len); - data = mp_encode_str(data, sql_str, sql_str_len); - sqlDbFree(db, sql_str); - - trigger_name = sqlDbStrDup(db, trigger_name); - if (trigger_name == NULL) { - sqlDbFree(db, opts_buff); - goto cleanup; - } - - sqlVdbeAddOp4(v, OP_String8, 0, first_col, 0, - trigger_name, P4_DYNAMIC); - sqlVdbeAddOp2(v, OP_Integer, trigger->space_id, - first_col + 1); - sqlVdbeAddOp4(v, OP_Blob, opts_buff_sz, first_col + 2, - SQL_SUBTYPE_MSGPACK, opts_buff, P4_DYNAMIC); - sqlVdbeAddOp3(v, OP_MakeRecord, first_col, 3, record); - sqlVdbeAddOp4(v, OP_IdxInsert, record, 0, 0, - (char *)_trigger, P4_SPACEPTR); - - sqlVdbeChangeP5(v, OPFLAG_NCHANGE); - - sql_set_multi_write(parse, false); - } else { - parse->parsed_ast.trigger = trigger; - parse->parsed_ast_type = AST_TYPE_TRIGGER; - trigger = NULL; - } - -cleanup: - sql_trigger_delete(db, trigger); - assert(parse->parsed_ast.trigger == NULL || parse->parse_only); - sqlDeleteTriggerStep(db, step_list); -} - struct TriggerStep * sql_trigger_select_step(struct sql *db, struct Select *select) { diff --git a/test/sql/triggers.result b/test/sql/triggers.result index 9dfe981a0..1a70ddf2b 100644 --- a/test/sql/triggers.result +++ b/test/sql/triggers.result @@ -34,7 +34,7 @@ immutable_part(box.space._trigger:select()) --- - - - T1T - {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO - t2 VALUES(1); END; '} + t2 VALUES(1); END'} ... space_id = box.space._space.index["name"]:get('T1').id --- @@ -68,7 +68,7 @@ immutable_part(box.space._trigger:select()) --- - - - T1T - {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO - t2 VALUES(1); END; '} + t2 VALUES(1); END'} ... box.execute("DROP TABLE T1;") --- @@ -90,7 +90,7 @@ immutable_part(box.space._trigger:select()) --- - - - T1T - {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO - t2 VALUES(1); END; '} + t2 VALUES(1); END'} ... space_id = box.space._space.index["name"]:get('T1').id --- @@ -129,7 +129,7 @@ immutable_part(box.space._trigger:select()) --- - - - T1T - {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO - t2 VALUES(1); END; '} + t2 VALUES(1); END'} - - T2T - {'sql': 'CREATE TRIGGER t2t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO t2 VALUES(2); END;'} @@ -167,7 +167,7 @@ immutable_part(box.space._trigger:select()) --- - - - T1T - {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO - t2 VALUES(1); END; '} + t2 VALUES(1); END'} ... box.execute("INSERT INTO t1 VALUES(3);") --- @@ -198,13 +198,13 @@ immutable_part(box.space._trigger:select()) --- - - - T1T - {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO - t2 VALUES(1); END; '} + t2 VALUES(1); END'} - - T2T - {'sql': 'CREATE TRIGGER t2t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO - t2 VALUES(2); END; '} + t2 VALUES(2); END'} - - T3T - {'sql': 'CREATE TRIGGER t3t AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO - t2 VALUES(3); END; '} + t2 VALUES(3); END'} ... box.execute("INSERT INTO t1 VALUES(4);") --- diff --git a/test/sql/upgrade.result b/test/sql/upgrade.result index f0997e17f..b5c4ad500 100644 --- a/test/sql/upgrade.result +++ b/test/sql/upgrade.result @@ -86,7 +86,7 @@ t1t.name t1t.opts --- - {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t FOR EACH ROW BEGIN INSERT INTO t_out - VALUES(1); END;'} + VALUES(1); END'} ... t2t.name --- @@ -95,7 +95,7 @@ t2t.name t2t.opts --- - {'sql': 'CREATE TRIGGER t2t AFTER INSERT ON t FOR EACH ROW BEGIN INSERT INTO t_out - VALUES(2); END;'} + VALUES(2); END'} ... assert(t1t.space_id == t2t.space_id) --- -- 2.23.0