Re: [Tarantool-patches] [PATCH v4 2/5] sql: refactor create_table_def and parse

[Nikita Pettik <korablev@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 <ALTER TABLE ADD COLUMN>.
> 
> Needed for #3075
> ---
>  
> +/**
> + * Emit code to create sequences, indexes, check and foreign key
> + * constraints appeared in <CREATE TABLE>.
> + */
> +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;
> +};
> +