[Tarantool-patches] [PATCH v4 2/6] sql: refactor create_table_def and parse
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Sep 17 17:43:53 MSK 2020
Hi! Thanks for the patch!
On 11.09.2020 23:51, Roman Khabibov wrote:
> Move ck, fk constraint lists and autoincrement info from
> struct create_table_def to struct Parse
1. You moved them into new defs, not into struct Parse. You need
to revisit some commit messages in this branch after review fixes
are done.
> to make the code more
> reusable when implementing <ALTER TABLE ADD COLUMN>.
>
> Needed for #3075
> ---
> 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;
*. All struct memebers should have a comment. At least it is worth
documenting what are the members of checks and fkeys lists.
> };
>
> struct create_view_def {
> @@ -500,12 +501,12 @@ create_view_def_init(struct create_view_def *view_def, struct Token *name,
> }
>
> static inline void
> -create_table_def_destroy(struct create_table_def *table_def)
> +fkeys_def_destroy(struct create_fkeys_def *fkeys_def)
2. Methods of a struct should have a prefix with the struct name.
It should be create_fkeys_def_destroy().
> {
> - if (table_def->new_space == NULL)
> + if (fkeys_def->count == 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);
> }
>
> 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 <CREATE TABLE>.
> + */
> + struct create_fkeys_def fkeys_def;
> + struct create_checks_def checks_def;
3. fkeys_def -> create_fkeys_def.
checks_def -> create_checks_def.
> + /*
> + * True, if column within a <CREATE TABLE> statement to be
> + * created has <AUTOINCREMENT>.
> + */
> + bool has_autoinc;
> + /* Id of field with <AUTOINCREMENT>. */
> + uint32_t autoinc_fieldno;
> bool initiateTTrans; /* Initiate Tarantool transaction */
> /** If set - do not emit byte code at all, just parse. */
> bool parse_only;
>
More information about the Tarantool-patches
mailing list