From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 D4559469719 for ; Tue, 29 Sep 2020 02:29:06 +0300 (MSK) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.1\)) From: Roman Khabibov In-Reply-To: <20200916132708.GB10599@tarantool.org> Date: Tue, 29 Sep 2020 02:29:05 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <56316DCA-AF76-4E43-B5C3-CE41049C9329@tarantool.org> References: <20200911215115.6622-1-roman.habibov@tarantool.org> <20200911215115.6622-3-roman.habibov@tarantool.org> <20200916132708.GB10599@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 2/6] sql: refactor create_table_def and parse List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Hi! > On Sep 16, 2020, at 16:27, Nikita Pettik = wrote: >=20 > On 12 Sep 00:51, Roman Khabibov wrote: >> Move ck, fk constraint lists and autoincrement info from >> struct create_table_def to struct Parse to make the code more >> reusable when implementing . >>=20 >> Needed for #3075 >> --- >=20 > Does ANSI allow to include constraints in ADD COLUMN statement? > Why did you decide to add autoincrement to this list? Was there any > discussion on this subj? The standard says that the column descriptions in and are exactly the same. All consrtants, and are = allowed. But is not described in the standard. I suggested (Vlad didn't mind) to continue this logic and support inside , because there are no significant restrictions on this. We can add a sequence to an already existing table. >> src/box/sql/build.c | 182 = ++++++++++++++++++++++------------------ >> src/box/sql/parse.y | 2 + >> src/box/sql/parse_def.h | 53 ++++++------ >> src/box/sql/prepare.c | 3 +- >> src/box/sql/sqlInt.h | 12 +++ >> 5 files changed, 142 insertions(+), 110 deletions(-) >>=20 commit 19e3d75b0c2702fd931244cc4c0a032a5cc67c58 Author: Roman Khabibov Date: Sat Aug 1 00:27:52 2020 +0300 sql: refactor create_table_def and parse =20 Move ck, fk constraint lists and autoincrement info from struct create_table_def to struct Parse to make the code more reusable when implementing . =20 Needed for #3075 diff --git a/src/box/sql/build.c b/src/box/sql/build.c index d55c1cd71..6cc76bb77 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -590,7 +590,7 @@ sql_create_check_contraint(struct Parse *parser) } } else { assert(! is_alter); - uint32_t ck_idx =3D = ++parser->create_table_def.check_count; + uint32_t ck_idx =3D ++parser->checks_def.count; name =3D tt_sprintf("ck_unnamed_%s_%d", = space->def->name, ck_idx); } size_t name_len =3D strlen(name); @@ -652,8 +652,7 @@ sql_create_check_contraint(struct Parse *parser) sqlVdbeCountChanges(v); sqlVdbeChangeP5(v, OPFLAG_NCHANGE); } else { - rlist_add_entry(&parser->create_table_def.new_check, = ck_parse, - link); + rlist_add_entry(&parser->checks_def.checks, ck_parse, = link); } } =20 @@ -930,7 +929,7 @@ emitNewSysSequenceRecord(Parse *pParse, int = reg_seq_id, const char *seq_name) static int emitNewSysSpaceSequenceRecord(Parse *pParse, int reg_space_id, int = reg_seq_id) { - uint32_t fieldno =3D pParse->create_table_def.autoinc_fieldno; + uint32_t fieldno =3D pParse->autoinc_fieldno; =20 Vdbe *v =3D sqlGetVdbe(pParse); int first_col =3D pParse->nMem + 1; @@ -1147,6 +1146,88 @@ resolve_link(struct Parse *parse_context, const = struct space_def *def, return -1; } =20 +/** + * Emit code to create sequences, indexes, check and foreign key + * constraints appeared in . + */ +static void +sql_vdbe_create_constraints(struct Parse *parse, int reg_space_id) +{ + assert(reg_space_id !=3D 0); + struct space *space =3D parse->create_table_def.new_space; + assert(space !=3D NULL); + uint32_t i =3D 0; + for (; i < space->index_count; ++i) { + struct index *idx =3D space->index[i]; + vdbe_emit_create_index(parse, space->def, idx->def, + reg_space_id, idx->def->iid); + } + + /* + * Check to see if we need to create an _sequence table + * for keeping track of autoincrement keys. + */ + if (parse->has_autoinc) { + /* Do an insertion into _sequence. */ + int reg_seq_id =3D ++parse->nMem; + struct Vdbe *v =3D sqlGetVdbe(parse); + assert(v !=3D NULL); + sqlVdbeAddOp2(v, OP_NextSequenceId, 0, reg_seq_id); + int reg_seq_rec =3D emitNewSysSequenceRecord(parse, = reg_seq_id, + = space->def->name); + sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, = reg_seq_rec); + /* Do an insertion into _space_sequence. */ + int reg_space_seq_record =3D + emitNewSysSpaceSequenceRecord(parse, = reg_space_id, + reg_seq_id); + 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; + rlist_foreach_entry(fk_parse, &parse->fkeys_def.fkeys, link) { + struct fk_constraint_def *fk_def =3D fk_parse->fk_def; + if (fk_parse->selfref_cols !=3D NULL) { + struct ExprList *cols =3D = fk_parse->selfref_cols; + for (uint32_t i =3D 0; i < fk_def->field_count; = ++i) { + if (resolve_link(parse, space->def, + cols->a[i].zName, + = &fk_def->links[i].parent_field, + fk_def->name) !=3D 0) + return; + } + fk_def->parent_id =3D reg_space_id; + } else if (fk_parse->is_self_referenced) { + struct key_def *pk_key_def =3D + = sql_space_primary_key(space)->def->key_def; + if (pk_key_def->part_count !=3D = fk_def->field_count) { + diag_set(ClientError, = ER_CREATE_FK_CONSTRAINT, + fk_def->name, "number of = columns in "\ + "foreign key does not match the = "\ + "number of columns in the = primary "\ + "index of referenced table"); + parse->is_aborted =3D true; + return; + } + for (uint32_t i =3D 0; i < fk_def->field_count; = ++i) { + fk_def->links[i].parent_field =3D + pk_key_def->parts[i].fieldno; + } + fk_def->parent_id =3D reg_space_id; + } + fk_def->child_id =3D reg_space_id; + vdbe_emit_fk_constraint_create(parse, fk_def, = space->def->name); + } + + /* Code creation of CK constraints, if any. */ + struct ck_constraint_parse *ck_parse; + rlist_foreach_entry(ck_parse, &parse->checks_def.checks, link) { + vdbe_emit_ck_constraint_create(parse, ck_parse->ck_def, + reg_space_id, = space->def->name); + } +} + /* * This routine is called to report the final ")" that terminates * a CREATE TABLE statement. @@ -1213,73 +1294,7 @@ sqlEndTable(struct Parse *pParse) =20 int reg_space_id =3D getNewSpaceId(pParse); vdbe_emit_space_create(pParse, reg_space_id, name_reg, = new_space); - for (uint32_t i =3D 0; i < new_space->index_count; ++i) { - struct index *idx =3D new_space->index[i]; - vdbe_emit_create_index(pParse, new_space->def, idx->def, - reg_space_id, idx->def->iid); - } - - /* - * Check to see if we need to create an _sequence table - * for keeping track of autoincrement keys. - */ - if (pParse->create_table_def.has_autoinc) { - assert(reg_space_id !=3D 0); - /* Do an insertion into _sequence. */ - int reg_seq_id =3D ++pParse->nMem; - sqlVdbeAddOp2(v, OP_NextSequenceId, 0, reg_seq_id); - int reg_seq_record =3D - emitNewSysSequenceRecord(pParse, reg_seq_id, - new_space->def->name); - sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, = reg_seq_record); - /* Do an insertion into _space_sequence. */ - int reg_space_seq_record =3D - emitNewSysSpaceSequenceRecord(pParse, = reg_space_id, - reg_seq_id); - 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; - rlist_foreach_entry(fk_parse, = &pParse->create_table_def.new_fkey, - link) { - struct fk_constraint_def *fk_def =3D fk_parse->fk_def; - if (fk_parse->selfref_cols !=3D NULL) { - struct ExprList *cols =3D = fk_parse->selfref_cols; - for (uint32_t i =3D 0; i < fk_def->field_count; = ++i) { - if (resolve_link(pParse, new_space->def, - cols->a[i].zName, - = &fk_def->links[i].parent_field, - fk_def->name) !=3D 0) - return; - } - fk_def->parent_id =3D reg_space_id; - } else if (fk_parse->is_self_referenced) { - struct index *pk =3D = sql_space_primary_key(new_space); - if (pk->def->key_def->part_count !=3D = fk_def->field_count) { - diag_set(ClientError, = ER_CREATE_FK_CONSTRAINT, - fk_def->name, "number of = columns in "\ - "foreign key does not match the = "\ - "number of columns in the = primary "\ - "index of referenced table"); - pParse->is_aborted =3D true; - return; - } - for (uint32_t i =3D 0; i < fk_def->field_count; = ++i) { - fk_def->links[i].parent_field =3D - = pk->def->key_def->parts[i].fieldno; - } - fk_def->parent_id =3D reg_space_id; - } - fk_def->child_id =3D reg_space_id; - vdbe_emit_fk_constraint_create(pParse, fk_def, = space_name_copy); - } - struct ck_constraint_parse *ck_parse; - rlist_foreach_entry(ck_parse, = &pParse->create_table_def.new_check, - link) { - vdbe_emit_ck_constraint_create(pParse, ck_parse->ck_def, - reg_space_id, = space_name_copy); - } + sql_vdbe_create_constraints(pParse, reg_space_id); } =20 void @@ -1893,7 +1908,8 @@ sql_create_foreign_key(struct Parse = *parse_context) goto tnt_error; } memset(fk_parse, 0, sizeof(*fk_parse)); - rlist_add_entry(&table_def->new_fkey, fk_parse, link); + rlist_add_entry(&parse_context->fkeys_def.fkeys, = fk_parse, + link); } struct Token *parent =3D create_fk_def->parent_name; assert(parent !=3D NULL); @@ -1910,8 +1926,9 @@ sql_create_foreign_key(struct Parse = *parse_context) struct space *parent_space =3D space_by_name(parent_name); if (parent_space =3D=3D NULL) { if (is_self_referenced) { + struct rlist *fkeys =3D = &parse_context->fkeys_def.fkeys; struct fk_constraint_parse *fk =3D - rlist_first_entry(&table_def->new_fkey, + rlist_first_entry(fkeys, struct = fk_constraint_parse, link); fk->selfref_cols =3D parent_cols; @@ -1926,7 +1943,7 @@ sql_create_foreign_key(struct Parse = *parse_context) constraint_name =3D sqlMPrintf(db, "fk_unnamed_%s_%d", space->def->name, - ++table_def->fkey_count); + = ++parse_context->fkeys_def.count); } else { constraint_name =3D sql_name_from_token(db, = &create_def->name); @@ -2042,7 +2059,7 @@ sql_create_foreign_key(struct Parse = *parse_context) */ if (!is_alter) { struct fk_constraint_parse *fk_parse =3D - rlist_first_entry(&table_def->new_fkey, + = rlist_first_entry(&parse_context->fkeys_def.fkeys, struct fk_constraint_parse, = link); fk_parse->fk_def =3D fk_def; } else { @@ -2065,12 +2082,11 @@ tnt_error: void fk_constraint_change_defer_mode(struct Parse *parse_context, bool = is_deferred) { - if (parse_context->db->init.busy || - rlist_empty(&parse_context->create_table_def.new_fkey)) + struct rlist *fkeys =3D &parse_context->fkeys_def.fkeys; + if (parse_context->db->init.busy || rlist_empty(fkeys)) return; - rlist_first_entry(&parse_context->create_table_def.new_fkey, - struct fk_constraint_parse, = link)->fk_def->is_deferred =3D - is_deferred; + rlist_first_entry(fkeys, struct fk_constraint_parse, + link)->fk_def->is_deferred =3D is_deferred; } =20 /** @@ -3306,15 +3322,15 @@ vdbe_emit_halt_with_presence_test(struct Parse = *parser, int space_id, int sql_add_autoincrement(struct Parse *parse_context, uint32_t fieldno) { - if (parse_context->create_table_def.has_autoinc) { + if (parse_context->has_autoinc) { diag_set(ClientError, ER_SQL_SYNTAX_WITH_POS, parse_context->line_count, = parse_context->line_pos, "table must feature at most one AUTOINCREMENT = field"); parse_context->is_aborted =3D true; return -1; } - parse_context->create_table_def.has_autoinc =3D true; - parse_context->create_table_def.autoinc_fieldno =3D fieldno; + parse_context->has_autoinc =3D true; + parse_context->autoinc_fieldno =3D fieldno; return 0; } =20 diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 995875566..d81045d50 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -181,6 +181,8 @@ cmd ::=3D ROLLBACK TO savepoint_opt nm(X). { cmd ::=3D create_table create_table_args with_opts create_table_end. create_table ::=3D createkw TABLE ifnotexists(E) nm(Y). { create_table_def_init(&pParse->create_table_def, &Y, E); + create_checks_def_init(&pParse->checks_def); + create_fkeys_def_init(&pParse->fkeys_def); pParse->create_table_def.new_space =3D sqlStartTable(pParse, &Y); pParse->initiateTTrans =3D true; } diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h index cb0ecd2fc..c44e3dbbe 100644 --- a/src/box/sql/parse_def.h +++ b/src/box/sql/parse_def.h @@ -205,26 +205,16 @@ struct create_entity_def { struct create_table_def { struct create_entity_def base; struct space *new_space; - /** - * Number of FK constraints declared within - * CREATE TABLE statement. - */ - uint32_t fkey_count; - /** - * Foreign key constraint appeared in CREATE TABLE stmt. - */ - struct rlist new_fkey; - /** - * Number of CK constraints declared within - * CREATE TABLE statement. - */ - uint32_t check_count; - /** Check constraint appeared in CREATE TABLE stmt. */ - struct rlist new_check; - /** True, if table to be created has AUTOINCREMENT PK. */ - bool has_autoinc; - /** Id of field with AUTOINCREMENT. */ - uint32_t autoinc_fieldno; +}; + +struct create_checks_def { + struct rlist checks; + uint32_t count; +}; + +struct create_fkeys_def { + struct rlist fkeys; + uint32_t count; }; =20 struct create_view_def { @@ -482,9 +472,20 @@ create_table_def_init(struct create_table_def = *table_def, struct Token *name, { create_entity_def_init(&table_def->base, ENTITY_TYPE_TABLE, = NULL, name, if_not_exists); - rlist_create(&table_def->new_fkey); - rlist_create(&table_def->new_check); - table_def->autoinc_fieldno =3D 0; +} + +static inline void +create_checks_def_init(struct create_checks_def *checks_def) +{ + rlist_create(&checks_def->checks); + checks_def->count =3D 0; +} + +static inline void +create_fkeys_def_init(struct create_fkeys_def *fkeys_def) +{ + rlist_create(&fkeys_def->fkeys); + fkeys_def->count =3D 0; } =20 static inline void @@ -500,12 +501,12 @@ create_view_def_init(struct create_view_def = *view_def, struct Token *name, } =20 static inline void -create_table_def_destroy(struct create_table_def *table_def) +fkeys_def_destroy(struct create_fkeys_def *fkeys_def) { - if (table_def->new_space =3D=3D NULL) + if (fkeys_def->count =3D=3D 0) return; struct fk_constraint_parse *fk; - rlist_foreach_entry(fk, &table_def->new_fkey, link) + rlist_foreach_entry(fk, &fkeys_def->fkeys, link) sql_expr_list_delete(sql_get(), fk->selfref_cols); } =20 diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c index a5a258805..2eab12a0b 100644 --- a/src/box/sql/prepare.c +++ b/src/box/sql/prepare.c @@ -200,6 +200,7 @@ sql_parser_create(struct Parse *parser, struct sql = *db, uint32_t sql_flags) parser->sql_flags =3D sql_flags; parser->line_count =3D 1; parser->line_pos =3D 1; + parser->has_autoinc =3D false; region_create(&parser->region, &cord()->slabc); } =20 @@ -211,7 +212,7 @@ sql_parser_destroy(Parse *parser) sql *db =3D parser->db; sqlDbFree(db, parser->aLabel); sql_expr_list_delete(db, parser->pConstExpr); - create_table_def_destroy(&parser->create_table_def); + fkeys_def_destroy(&parser->fkeys_def); if (db !=3D NULL) { assert(db->lookaside.bDisable >=3D parser->disableLookaside); diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 5913d7614..7057c20b6 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -2257,6 +2257,18 @@ struct Parse { * sqlEndTable() function). */ struct create_table_def create_table_def; + /* + * FK and CK constraints appeared in a . + */ + struct create_fkeys_def fkeys_def; + struct create_checks_def checks_def; + /* + * True, if column within a statement to be + * created has . + */ + bool has_autoinc; + /* Id of field with . */ + uint32_t autoinc_fieldno; bool initiateTTrans; /* Initiate Tarantool transaction */ /** If set - do not emit byte code at all, just parse. */ bool parse_only;