From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 B2E71469719 for ; Fri, 6 Nov 2020 01:17:31 +0300 (MSK) Date: Thu, 5 Nov 2020 22:17:28 +0000 From: Nikita Pettik Message-ID: <20201105221728.GA8188@tarantool.org> References: <20201009134529.13212-1-roman.habibov@tarantool.org> <20201009134529.13212-3-roman.habibov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20201009134529.13212-3-roman.habibov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 2/5] sql: refactor create_table_def and parse List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org On 09 Oct 16:45, Roman Khabibov wrote: > Move ck, fk constraint lists from struct create_table_def into new > defs and autoincrement into struct Parse to make the code more > reusable when implementing . > > Needed for #3075 > --- > > +/** > + * Emit code to create sequences, indexes, check and foreign key > + * constraints appeared in . > + */ > +static void > +vdbe_emit_create_constraints(struct Parse *parse, int reg_space_id) I'd move this refactoring in a separate patch. Up to you. > @@ -3306,15 +3324,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 = true; > return -1; > } > - parse_context->create_table_def.has_autoinc = true; > - parse_context->create_table_def.autoinc_fieldno = fieldno; > + parse_context->has_autoinc = true; > + parse_context->autoinc_fieldno = fieldno; > return 0; > } > > diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h > index cb0ecd2fc..21829b6f0 100644 > --- a/src/box/sql/parse_def.h > +++ b/src/box/sql/parse_def.h > @@ -205,26 +205,20 @@ 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; Did you consider adding create_column_def class? Imho it would fit better in parse hierarchy: it would derive from create_entity_def, has field_def, autoinc, ck, fk members. > +}; > + > +struct create_checks_def { Why not create_ck_constraint_def? This naming would be more consistent with existing ck_contraint_def etc. The same for create_fk_constraint_def. > + /** List of ck_constraint_parse objects. */ > + struct rlist checks; > + /** Count of ck_constraint_parse objects. */ > + uint32_t count; > +}; > +