From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 52939469719 for ; Mon, 9 Mar 2020 18:08:58 +0300 (MSK) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\)) From: Roman Khabibov In-Reply-To: <20200131150537.90826-1-roman.habibov@tarantool.org> Date: Mon, 9 Mar 2020 18:08:56 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <7F7A0DA7-C5F2-43FD-AFF4-F6F4C700D603@tarantool.org> References: <20200131150537.90826-1-roman.habibov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] sql: support column addition List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: Vladislav Shpilevoy Vlad, can you take it, please? > On Jan 31, 2020, at 18:05, Roman Khabibov = wrote: >=20 > Enable to add column to existing space with > statement. Column definition can be > supplemented with , clauses and <[NOT] NULL> > column constraint. >=20 > Closes #3075 >=20 > @TarantoolBot document > Title: Add columns to existing tables in SQL > Now, it is possible to add columns to existing empty spaces using > > statement. The column definition is the same as in > statement, except that table constraints (PRIMARY KEY, UNIQUE, > REFERENCES, CHECK) cannot be specified yet. >=20 > For example: >=20 > tarantool> box.execute([[CREATE TABLE test ( > a INTEGER PRIMARY KEY > );]]) > --- > - row_count: 1 > ... >=20 > tarantool> box.execute([[ALTER TABLE test ADD COLUMN > b TEXT > NOT NULL > DEFAULT ('a') > COLLATE "unicode_ci" > ;]]) > --- > - row_count: 0 > ... > --- > Branch: = https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3075-add-colum= n > Issue: https://github.com/tarantool/tarantool/issues/3075 >=20 > P.S.: I didn't implement column constraints in ADD COLUMN statement, > because of the sql_create_index(), sql_create_foreign_key() and > sql_create_check_contraint() functions. These functions have > branching inside: >=20 > 1) CREATE TABLE (parse->create_table_def.new_space !=3D NULL) > We work with the ephemeral space. Use its def, indexes array, > ck and fk rlists for checks, opcode emitting, etc and modify it on > parsing level. >=20 > 2) ALTER TABLE (parse->create_table_def.new_space =3D=3D NULL) > We work with existing space =3D space_by_name(), but don't modify it. > This space will be modified after parsing by alter.cc triggers. >=20 > In the case of > ALTER TABLE test ADD a INT PRIMARY KEY CONSTRAINT c UNIQUE CONSTRAINT = d UNIQUE ... > we need to modifiy the existing space, for example UNIQUE constraint > creation/opcode emitting requires new space def with new modified > fields array and new index_id_max. The same is for REFERENCES > constraint. With CHECK it seems easier, but I didn't implement it, > because I want to do patch in the futue: "Enable constraints in ADD = COLUMN", > if it will be needed. >=20 > There is obvious way to implement constraints parsing during > ADD COLUMN. To do partial copy of space =3D space_by_name() on region, > to keep it in parse->create_column_>def and to work with it. Then > we will have to change the current code a bit, but it is necessary > to write API to create an ephemeral space based on the existing. >=20 > I would like to hear your opinion. Maybe I'm generally wrong in > thinking about all this, and it could be made easier. >=20 > P.P.S.: Could you, please, help me with COLUMN keyword support. > I couldn't add it as I wanted, i.e., the line in parse.y near the > line 1754: > column_name(N) ::=3D COLUMN nm(A). { N =3D A; } > because I had: > /Users/r.khabibov/tarantool/src/box/sql/parse.h:171:9: error: = 'TK_COLUMN' macro redefined > #define TK_COLUMN >=20 > I don't understand why it is so? For example, the definition of > CONSTRAINT keyword in mkkeywordhash.c is the same, but there is no > error about this #define during compilation. >=20 > { "COLUMN", "TK_COLUMN", true }, > { "CONSTRAINT", "TK_CONSTRAINT", true }, >=20 > extra/mkkeywordhash.c | 6 +- > src/box/errcode.h | 1 + > src/box/sql.c | 28 +++++--- > src/box/sql/alter.c | 125 ++++++++++++++++++++++++++++++++++++ > src/box/sql/build.c | 106 ++++++++++++++++++------------ > src/box/sql/parse.y | 59 +++++++++++++++-- > src/box/sql/parse_def.h | 23 +++++++ > src/box/sql/sqlInt.h | 40 ++++++++++++ > src/box/sql/tarantoolInt.h | 6 +- > test/box/misc.result | 1 + > test/sql-tap/alter.test.lua | 76 +++++++++++++++++++++- > 11 files changed, 405 insertions(+), 66 deletions(-) >=20 > diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c > index 006285622..51c8cbb18 100644 > --- a/extra/mkkeywordhash.c > +++ b/extra/mkkeywordhash.c > @@ -75,11 +75,7 @@ static Keyword aKeywordTable[] =3D { > { "CAST", "TK_CAST", false }, > { "CHECK", "TK_CHECK", true }, > { "COLLATE", "TK_COLLATE", true }, > - /* gh-3075: Reserved until ALTER ADD COLUMN is implemeneted. > - * Move it back to ALTER when done. > - */ > - /* { "COLUMN", "TK_COLUMNKW", true }, */ > - { "COLUMN", "TK_STANDARD", true }, > + { "COLUMN", "TK_COLUMN", true }, > { "COMMIT", "TK_COMMIT", true }, > { "CONFLICT", "TK_CONFLICT", false }, > { "CONSTRAINT", "TK_CONSTRAINT", true }, > diff --git a/src/box/errcode.h b/src/box/errcode.h > index 3065b1948..e076edf02 100644 > --- a/src/box/errcode.h > +++ b/src/box/errcode.h > @@ -264,6 +264,7 @@ struct errcode_record { > /*209 */_(ER_SESSION_SETTING_INVALID_VALUE, "Session setting = %s expected a value of type %s") \ > /*210 */_(ER_SQL_PREPARE, "Failed to prepare SQL = statement: %s") \ > /*211 */_(ER_WRONG_QUERY_ID, "Prepared statement with = id %u does not exist") \ > + /*212 */_(ER_SQL_CANT_ADD_COLUMN_TO_VIEW, "Can't add = column '%s' to the view '%s'") \ >=20 > /* > * !IMPORTANT! Please follow instructions at start of the file > diff --git a/src/box/sql.c b/src/box/sql.c > index 1256df856..7157cabe6 100644 > --- a/src/box/sql.c > +++ b/src/box/sql.c > @@ -923,7 +923,8 @@ cursor_advance(BtCursor *pCur, int *pRes) > */ >=20 > char * > -sql_encode_table(struct region *region, struct space_def *def, = uint32_t *size) > +sql_encode_table_format(struct region *region, struct field_def = *fields, > + uint32_t field_count, uint32_t *size) > { > size_t used =3D region_used(region); > struct mpstream stream; > @@ -931,12 +932,11 @@ sql_encode_table(struct region *region, struct = space_def *def, uint32_t *size) > mpstream_init(&stream, region, region_reserve_cb, = region_alloc_cb, > set_encode_error, &is_error); >=20 > - assert(def !=3D NULL); > - uint32_t field_count =3D def->field_count; > + assert(fields !=3D NULL); > mpstream_encode_array(&stream, field_count); > for (uint32_t i =3D 0; i < field_count && !is_error; i++) { > - uint32_t cid =3D def->fields[i].coll_id; > - struct field_def *field =3D &def->fields[i]; > + struct field_def *field =3D &fields[i]; > + uint32_t cid =3D field->coll_id; > const char *default_str =3D field->default_value; > int base_len =3D 4; > if (cid !=3D COLL_NONE) > @@ -947,16 +947,16 @@ sql_encode_table(struct region *region, struct = space_def *def, uint32_t *size) > mpstream_encode_str(&stream, "name"); > mpstream_encode_str(&stream, field->name); > mpstream_encode_str(&stream, "type"); > - assert(def->fields[i].is_nullable =3D=3D > - = action_is_nullable(def->fields[i].nullable_action)); > + assert(field->is_nullable =3D=3D > + action_is_nullable(field->nullable_action)); > mpstream_encode_str(&stream, = field_type_strs[field->type]); > mpstream_encode_str(&stream, "is_nullable"); > - mpstream_encode_bool(&stream, = def->fields[i].is_nullable); > + mpstream_encode_bool(&stream, field->is_nullable); > mpstream_encode_str(&stream, "nullable_action"); >=20 > - assert(def->fields[i].nullable_action < = on_conflict_action_MAX); > + assert(field->nullable_action < on_conflict_action_MAX); > const char *action =3D > - = on_conflict_action_strs[def->fields[i].nullable_action]; > + on_conflict_action_strs[field->nullable_action]; > mpstream_encode_str(&stream, action); > if (cid !=3D COLL_NONE) { > mpstream_encode_str(&stream, "collation"); > @@ -980,6 +980,14 @@ sql_encode_table(struct region *region, struct = space_def *def, uint32_t *size) > return raw; > } >=20 > +char * > +sql_encode_table(struct region *region, struct space_def *def, = uint32_t *size) > +{ > + assert(def !=3D NULL); > + return sql_encode_table_format(region, def->fields, = def->field_count, > + size); > +} > + > char * > sql_encode_table_opts(struct region *region, struct space_def *def, > uint32_t *size) > diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c > index 973b420cf..c12d9373a 100644 > --- a/src/box/sql/alter.c > +++ b/src/box/sql/alter.c > @@ -36,6 +36,7 @@ > #include "sqlInt.h" > #include "box/box.h" > #include "box/schema.h" > +#include "tarantoolInt.h" >=20 > void > sql_alter_table_rename(struct Parse *parse) > @@ -209,3 +210,127 @@ rename_trigger(sql *db, char const *sql_stmt, = char const *table_name, > table_name, tname.z + tname.n); > return new_sql_stmt; > } > + > +void > +sql_alter_add_column_start(struct Parse *parse) > +{ > + struct create_column_def *create_column_def =3D = &parse->create_column_def; > + struct alter_entity_def *alter_entity_def =3D > + &create_column_def->base.base; > + assert(alter_entity_def->entity_type =3D=3D ENTITY_TYPE_COLUMN); > + assert(alter_entity_def->alter_action =3D=3D = ALTER_ACTION_CREATE); > + > + const char *space_name =3D > + alter_entity_def->entity_name->a[0].zName; > + struct space *space =3D space_by_name(space_name); > + struct sql *db =3D parse->db; > + if (space =3D=3D NULL) { > + diag_set(ClientError, ER_NO_SUCH_SPACE, space_name); > + goto tnt_error; > + } > + > + struct space_def *def =3D space->def; > + create_column_def->space_def =3D def; > + if (sql_space_def_check_format(def) !=3D 0) > + goto tnt_error; > + uint32_t new_field_count =3D def->field_count + 1; > + create_column_def->new_field_count =3D new_field_count; > + > +#if SQL_MAX_COLUMN > + if ((int)new_field_count > db->aLimit[SQL_LIMIT_COLUMN]) { > + diag_set(ClientError, ER_SQL_COLUMN_COUNT_MAX, = def->name, > + new_field_count, db->aLimit[SQL_LIMIT_COLUMN]); > + goto tnt_error; > + } > +#endif > + /* > + * Create new fields array and add the new column's > + * field_def to its end. During copying don't make > + * duplicates of the non-scalar fields (name, > + * default_value, default_value_expr), because they will > + * be just copied and encoded to the msgpack array on sql > + * allocator in sql_alter_add_column_end(). > + */ > + uint32_t size =3D sizeof(struct field_def) * new_field_count; > + struct region *region =3D &parse->region; > + struct field_def *new_fields =3D region_alloc(region, size); > + if (new_fields =3D=3D NULL) { > + diag_set(OutOfMemory, size, "region_alloc", = "new_fields"); > + goto tnt_error; > + } > + memcpy(new_fields, def->fields, > + sizeof(struct field_def) * def->field_count); > + create_column_def->new_fields =3D new_fields; > + struct field_def *new_column_def =3D = &new_fields[def->field_count]; > + > + memcpy(new_column_def, &field_def_default, sizeof(struct = field_def)); > + struct Token *name =3D &create_column_def->base.name; > + new_column_def->name =3D > + sql_normalized_name_region_new(region, name->z, = name->n); > + if (new_column_def->name =3D=3D NULL) > + goto tnt_error; > + if (def->opts.is_view) { > + diag_set(ClientError, ER_SQL_CANT_ADD_COLUMN_TO_VIEW, > + new_column_def->name, def->name); > + goto tnt_error; > + } > + > + new_column_def->type =3D create_column_def->type_def->type; > + > +exit_alter_add_column_start: > + sqlSrcListDelete(db, alter_entity_def->entity_name); > + return; > +tnt_error: > + parse->is_aborted =3D true; > + goto exit_alter_add_column_start; > +} > + > +void > +sql_alter_add_column_end(struct Parse *parse) > +{ > + struct create_column_def *create_column_def =3D = &parse->create_column_def; > + uint32_t new_field_count =3D create_column_def->new_field_count; > + uint32_t table_stmt_sz =3D 0; > + char *table_stmt =3D > + sql_encode_table_format(&parse->region, > + create_column_def->new_fields, > + new_field_count, = &table_stmt_sz); > + if (table_stmt =3D=3D NULL) { > + parse->is_aborted =3D true; > + return; > + } > + char *raw =3D sqlDbMallocRaw(parse->db, table_stmt_sz); > + if (raw =3D=3D NULL){ > + parse->is_aborted =3D true; > + return; > + } > + memcpy(raw, table_stmt, table_stmt_sz); > + > + struct space *system_space =3D space_by_id(BOX_SPACE_ID); > + assert(system_space !=3D NULL); > + int cursor =3D parse->nTab++; > + struct Vdbe *v =3D sqlGetVdbe(parse); > + assert(v !=3D NULL); > + vdbe_emit_open_cursor(parse, cursor, 0, system_space); > + sqlVdbeChangeP5(v, OPFLAG_SYSTEMSP); > + > + int key_reg =3D sqlGetTempReg(parse); > + sqlVdbeAddOp2(v, OP_Integer, create_column_def->space_def->id, = key_reg); > + int addr =3D sqlVdbeAddOp4Int(v, OP_Found, cursor, 0, key_reg, = 1); > + sqlVdbeAddOp2(v, OP_Halt, -1, ON_CONFLICT_ACTION_ABORT); > + sqlVdbeJumpHere(v, addr); > + > + const int system_space_field_count =3D 7; > + int tuple_reg =3D sqlGetTempRange(parse, = system_space_field_count + 1); > + for (int i =3D 0; i < system_space_field_count - 1; ++i) > + sqlVdbeAddOp3(v, OP_Column, cursor, i, tuple_reg + i); > + sqlVdbeAddOp1(v, OP_Close, cursor); > + > + sqlVdbeAddOp2(v, OP_Integer, new_field_count, tuple_reg + 4); > + sqlVdbeAddOp4(v, OP_Blob, table_stmt_sz, tuple_reg + 6, > + SQL_SUBTYPE_MSGPACK, raw, P4_DYNAMIC); > + sqlVdbeAddOp3(v, OP_MakeRecord, tuple_reg, = system_space_field_count, > + tuple_reg + system_space_field_count); > + sqlVdbeAddOp4(v, OP_IdxReplace, tuple_reg + = system_space_field_count, 0, > + 0, (char *)system_space, P4_SPACEPTR); > +} > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index bc50ecbfa..2f51b55df 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -368,11 +368,21 @@ void > sql_column_add_nullable_action(struct Parse *parser, > enum on_conflict_action nullable_action) > { > - struct space *space =3D parser->create_table_def.new_space; > - if (space =3D=3D NULL || NEVER(space->def->field_count < 1)) > + struct space_def *def =3D NULL; > + struct field_def *field =3D NULL; > + if (parser->create_table_def.new_space !=3D NULL) { > + def =3D parser->create_table_def.new_space->def; > + if (NEVER(def->field_count < 1)) > + return; > + field =3D &def->fields[def->field_count - 1]; > + } else if (parser->create_column_def.space_def !=3D NULL) { > + def =3D parser->create_column_def.space_def; > + uint32_t field_count =3D > + parser->create_column_def.new_field_count; > + field =3D = &parser->create_column_def.new_fields[field_count - 1]; > + } else { > return; > - struct space_def *def =3D space->def; > - struct field_def *field =3D &def->fields[def->field_count - 1]; > + } > if (field->nullable_action !=3D ON_CONFLICT_ACTION_DEFAULT && > nullable_action !=3D field->nullable_action) { > /* Prevent defining nullable_action many times. */ > @@ -390,51 +400,53 @@ sql_column_add_nullable_action(struct Parse = *parser, > } >=20 > /* > - * The expression is the default value for the most recently added = column > - * of the table currently under construction. > + * The expression is the default value for the most recently added > + * column. > * > * Default value expressions must be constant. Raise an exception if = this > * is not the case. > * > * This routine is called by the parser while in the middle of > - * parsing a CREATE TABLE statement. > + * parsing a or statement. > */ > void > sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan) > { > sql *db =3D pParse->db; > - struct space *p =3D pParse->create_table_def.new_space; > - if (p !=3D NULL) { > - assert(p->def->opts.is_ephemeral); > - struct space_def *def =3D p->def; > - if (!sqlExprIsConstantOrFunction > - (pSpan->pExpr, db->init.busy)) { > - const char *column_name =3D > - def->fields[def->field_count - 1].name; > - diag_set(ClientError, ER_CREATE_SPACE, = def->name, > - tt_sprintf("default value of column = '%s' is "\ > - "not constant", = column_name)); > + struct space_def *def =3D NULL; > + struct field_def *field =3D NULL; > + if (pParse->create_table_def.new_space !=3D NULL) { > + def =3D pParse->create_table_def.new_space->def; > + assert(def->opts.is_ephemeral); > + field =3D &def->fields[def->field_count - 1]; > + } else if (pParse->create_column_def.space_def !=3D NULL) { > + def =3D pParse->create_column_def.space_def; > + uint32_t field_count =3D > + pParse->create_column_def.new_field_count; > + field =3D = &pParse->create_column_def.new_fields[field_count - 1]; > + } else { > + goto add_default_value_exit; > + } > + if (!sqlExprIsConstantOrFunction(pSpan->pExpr, db->init.busy)) { > + diag_set(ClientError, ER_CREATE_SPACE, def->name, > + tt_sprintf("default value of column '%s' is not = " > + "constant", field->name)); > + pParse->is_aborted =3D true; > + } else { > + assert(def !=3D NULL); > + struct region *region =3D &pParse->region; > + uint32_t default_length =3D (int)(pSpan->zEnd - = pSpan->zStart); > + field->default_value =3D region_alloc(region, = default_length + 1); > + if (field->default_value =3D=3D NULL) { > + diag_set(OutOfMemory, default_length + 1, > + "region_alloc", = "field->default_value"); > pParse->is_aborted =3D true; > - } else { > - assert(def !=3D NULL); > - struct field_def *field =3D > - &def->fields[def->field_count - 1]; > - struct region *region =3D &pParse->region; > - uint32_t default_length =3D (int)(pSpan->zEnd - = pSpan->zStart); > - field->default_value =3D region_alloc(region, > - = default_length + 1); > - if (field->default_value =3D=3D NULL) { > - diag_set(OutOfMemory, default_length + = 1, > - "region_alloc", > - "field->default_value"); > - pParse->is_aborted =3D true; > - return; > - } > - strncpy(field->default_value, pSpan->zStart, > - default_length); > - field->default_value[default_length] =3D '\0'; > + return; > } > + strncpy(field->default_value, pSpan->zStart, = default_length); > + field->default_value[default_length] =3D '\0'; > } > +add_default_value_exit: > sql_expr_delete(db, pSpan->pExpr, false); > } >=20 > @@ -686,17 +698,27 @@ void > sqlAddCollateType(Parse * pParse, Token * pToken) > { > struct space *space =3D pParse->create_table_def.new_space; > - if (space =3D=3D NULL) > + uint32_t *coll_id =3D NULL; > + if (space !=3D NULL) { > + uint32_t i =3D space->def->field_count - 1; > + coll_id =3D &space->def->fields[i].coll_id; > + } else if (pParse->create_column_def.space_def !=3D NULL) { > + uint32_t field_count =3D > + pParse->create_column_def.new_field_count; > + struct field_def *field =3D > + = &pParse->create_column_def.new_fields[field_count - 1]; > + coll_id =3D &field->coll_id; > + } else { > return; > - uint32_t i =3D space->def->field_count - 1; > + } > sql *db =3D pParse->db; > char *coll_name =3D sql_name_from_token(db, pToken); > if (coll_name =3D=3D NULL) { > pParse->is_aborted =3D true; > return; > } > - uint32_t *coll_id =3D &space->def->fields[i].coll_id; > - if (sql_get_coll_seq(pParse, coll_name, coll_id) !=3D NULL) { > + if (sql_get_coll_seq(pParse, coll_name, coll_id) !=3D NULL && > + space !=3D NULL) { > /* If the column is declared as " PRIMARY KEY = COLLATE ", > * then an index may have been created on this column = before the > * collation type was added. Correct this if it is the = case. > @@ -858,7 +880,9 @@ vdbe_emit_space_create(struct Parse *pParse, int = space_id_reg, > if (table_opts_stmt =3D=3D NULL) > goto error; > uint32_t table_stmt_sz =3D 0; > - char *table_stmt =3D sql_encode_table(region, space->def, = &table_stmt_sz); > + char *table_stmt =3D sql_encode_table_format(region, = space->def->fields, > + = space->def->field_count, > + &table_stmt_sz); > if (table_stmt =3D=3D NULL) > goto error; > char *raw =3D sqlDbMallocRaw(pParse->db, > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y > index cfe1c0012..2eba72ac3 100644 > --- a/src/box/sql/parse.y > +++ b/src/box/sql/parse.y > @@ -289,10 +289,14 @@ carglist ::=3D . > %type cconsname { struct Token } > cconsname(N) ::=3D CONSTRAINT nm(X). { N =3D X; } > cconsname(N) ::=3D . { N =3D Token_nil; } > -ccons ::=3D DEFAULT term(X). = {sqlAddDefaultValue(pParse,&X);} > -ccons ::=3D DEFAULT LP expr(X) RP. = {sqlAddDefaultValue(pParse,&X);} > -ccons ::=3D DEFAULT PLUS term(X). = {sqlAddDefaultValue(pParse,&X);} > -ccons ::=3D DEFAULT MINUS(A) term(X). { > +ccons ::=3D default_clause_1. > +default_clause_1 ::=3D DEFAULT term(X). = {sqlAddDefaultValue(pParse,&X);} > +ccons ::=3D default_clause_2. > +default_clause_2 ::=3D DEFAULT LP expr(X) RP. = {sqlAddDefaultValue(pParse,&X);} > +ccons ::=3D default_clause_3. > +default_clause_3 ::=3D DEFAULT PLUS term(X). = {sqlAddDefaultValue(pParse,&X);} > +ccons ::=3D default_clause_4. > +default_clause_4 ::=3D DEFAULT MINUS(A) term(X). { > ExprSpan v; > v.pExpr =3D sqlPExpr(pParse, TK_UMINUS, X.pExpr, 0); > v.zStart =3D A.z; > @@ -303,13 +307,18 @@ ccons ::=3D DEFAULT MINUS(A) term(X). { > // In addition to the type name, we also care about the primary key = and > // UNIQUE constraints. > // > -ccons ::=3D NULL onconf(R). { > +ccons ::=3D null_cons. > +null_cons ::=3D NULL onconf(R). { > sql_column_add_nullable_action(pParse, ON_CONFLICT_ACTION_NONE); > /* Trigger nullability mismatch error if required. */ > if (R !=3D ON_CONFLICT_ACTION_ABORT) > sql_column_add_nullable_action(pParse, R); > } > -ccons ::=3D NOT NULL onconf(R). = {sql_column_add_nullable_action(pParse, R);} > +ccons ::=3D not_null_cons. > +not_null_cons ::=3D NOT NULL onconf(R). { > + sql_column_add_nullable_action(pParse, R); > +} > + > ccons ::=3D cconsname(N) PRIMARY KEY sortorder(Z). { > create_index_def_init(&pParse->create_index_def, NULL, &N, NULL, > SQL_INDEX_TYPE_CONSTRAINT_PK, Z, false); > @@ -335,7 +344,8 @@ ccons ::=3D cconsname(N) REFERENCES nm(T) = eidlist_opt(TA) matcharg(M) refargs(R). > sql_create_foreign_key(pParse); > } > ccons ::=3D defer_subclause(D). = {fk_constraint_change_defer_mode(pParse, D);} > -ccons ::=3D COLLATE id(C). {sqlAddCollateType(pParse, &C);} > +ccons ::=3D collate_clause. > +collate_clause ::=3D COLLATE id(C). {sqlAddCollateType(pParse, = &C);} >=20 > // The optional AUTOINCREMENT keyword > %type autoinc {int} > @@ -1729,11 +1739,46 @@ alter_table_start(A) ::=3D ALTER TABLE = fullname(T) . { A =3D T; } >=20 > %type alter_add_constraint {struct alter_args} > alter_add_constraint(A) ::=3D alter_table_start(T) ADD CONSTRAINT = nm(N). { > + A.table_name =3D T; > + A.name =3D N; > + pParse->initiateTTrans =3D true; > + } > + > +%type alter_add_column {struct alter_args} > +alter_add_column(A) ::=3D alter_table_start(T) ADD column_name(N). { > A.table_name =3D T; > A.name =3D N; > pParse->initiateTTrans =3D true; > } >=20 > +column_name(N) ::=3D nm(A). { N =3D A; } > + > +cmd ::=3D alter_column_def alter_carglist add_column_end. > + > +alter_column_def ::=3D alter_add_column(N) typedef(Y). { > + create_column_def_init(&pParse->create_column_def, N.table_name, = &N.name, &Y); > + sql_alter_add_column_start(pParse); > +} > + > +/* > + * "alter_carglist" is a list of additional constraints and > + * clauses that come after the column name and column type in a > + * ALTER TABLE ADD [COLUMN] statement. > + */ > +alter_carglist ::=3D alter_carglist alter_ccons. > +alter_carglist ::=3D . > +alter_ccons ::=3D default_clause_1. > +alter_ccons ::=3D default_clause_2. > +alter_ccons ::=3D default_clause_3. > +alter_ccons ::=3D default_clause_4. > +alter_ccons ::=3D null_cons. > +alter_ccons ::=3D not_null_cons. > +alter_ccons ::=3D collate_clause. > + > +add_column_end ::=3D . { > + sql_alter_add_column_end(pParse); > +} > + > cmd ::=3D alter_add_constraint(N) FOREIGN KEY LP eidlist(FA) RP = REFERENCES > nm(T) eidlist_opt(TA) matcharg(M) refargs(R) = defer_subclause_opt(D). { > create_fk_def_init(&pParse->create_fk_def, N.table_name, &N.name, = FA, &T, TA, > diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h > index 2f433e4c0..56887efa7 100644 > --- a/src/box/sql/parse_def.h > +++ b/src/box/sql/parse_def.h > @@ -154,6 +154,7 @@ enum sql_index_type { >=20 > enum entity_type { > ENTITY_TYPE_TABLE =3D 0, > + ENTITY_TYPE_COLUMN, > ENTITY_TYPE_VIEW, > ENTITY_TYPE_INDEX, > ENTITY_TYPE_TRIGGER, > @@ -227,6 +228,18 @@ struct create_table_def { > uint32_t autoinc_fieldno; > }; >=20 > +struct create_column_def { > + struct create_entity_def base; > + /** Column type. */ > + struct type_def *type_def; > + /** Already existing space def. */ > + struct space_def *space_def; > + /** New format array with the new column. */ > + struct field_def *new_fields; > + /** Old field count + 1. */ > + uint32_t new_field_count; > +}; > + > struct create_view_def { > struct create_entity_def base; > /** > @@ -486,6 +499,16 @@ create_table_def_init(struct create_table_def = *table_def, struct Token *name, > table_def->autoinc_fieldno =3D 0; > } >=20 > +static inline void > +create_column_def_init(struct create_column_def *column_def, > + struct SrcList *table_name, struct Token *name, > + struct type_def *type_def) > +{ > + create_entity_def_init(&column_def->base, ENTITY_TYPE_COLUMN, > + table_name, name, false); > + column_def->type_def =3D type_def; > +} > + > static inline void > create_view_def_init(struct create_view_def *view_def, struct Token = *name, > struct Token *create, struct ExprList *aliases, > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h > index d1fcf4761..46d3d6d2a 100644 > --- a/src/box/sql/sqlInt.h > +++ b/src/box/sql/sqlInt.h > @@ -2231,6 +2231,7 @@ struct Parse { > * from parse.y > */ > union { > + struct create_column_def create_column_def; > struct create_ck_def create_ck_def; > struct create_fk_def create_fk_def; > struct create_index_def create_index_def; > @@ -2842,6 +2843,18 @@ struct space * > sqlStartTable(Parse *, Token *); > void sqlAddColumn(Parse *, Token *, struct type_def *); >=20 > +/** > + * Set the is_nullable flag on the column @a field. > + * > + * @param field Column to set. > + * @param nullable_action on_conflict_action value. > + * @param space_name Name of the space to which the column > + * belongs. Needed to set error. > + * > + * @retval 0 Success. > + * @retval -1 nullable_action has been already set. > + */ > + > /** > * This routine is called by the parser while in the middle of > * parsing a CREATE TABLE statement. A "NOT NULL" constraint has > @@ -2868,6 +2881,19 @@ sqlAddPrimaryKey(struct Parse *parse); > void > sql_create_check_contraint(Parse *parser); >=20 > +/** > + * Add default value to the column @a field. > + * > + * @param parse Parsing context. > + * @param field Column to set. > + * @param span Default expression. > + * @param space_name Name of the space to which the column > + * belongs. Needed to set error. > + * > + * @retval 0 Success. > + * @retval -1 Error. > + */ > + > void sqlAddDefaultValue(Parse *, ExprSpan *); > void sqlAddCollateType(Parse *, Token *); >=20 > @@ -4031,6 +4057,20 @@ sql_alter_table_rename(struct Parse *parse); > void > sql_alter_ck_constraint_enable(struct Parse *parse); >=20 > +/** > + * Process the statement: do the checks of space and column > + * definition. Create new format array with a new column. > + */ > +void > +sql_alter_add_column_start(struct Parse *parse); > + > +/** > + * Encode new format array and emit code to update an entry in > + * _space. > + */ > +void > +sql_alter_add_column_end(struct Parse *parse); > + > /** > * Return the length (in bytes) of the token that begins at z[0]. > * Store the token type in *type before returning. > diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h > index 1ded6c709..5fd433c60 100644 > --- a/src/box/sql/tarantoolInt.h > +++ b/src/box/sql/tarantoolInt.h > @@ -118,14 +118,16 @@ tarantoolsqlIncrementMaxid(uint64_t = *space_max_id); > /** > * Encode format as entry to be inserted to _space on @region. > * @param region Region to use. > - * @param def Space definition to encode. > + * @param fields Space format. > + * @param field_count Number of fields. > * @param[out] size Size of result allocation. > * > * @retval NULL Error. > * @retval not NULL Pointer to msgpack on success. > */ > char * > -sql_encode_table(struct region *region, struct space_def *def, = uint32_t *size); > +sql_encode_table_format(struct region *region, struct field_def = *fields, > + uint32_t field_count, uint32_t *size); >=20 > /** > * Encode "opts" dictionary for _space entry on @region. > diff --git a/test/box/misc.result b/test/box/misc.result > index 3ca4e31ac..2aaec6566 100644 > --- a/test/box/misc.result > +++ b/test/box/misc.result > @@ -563,6 +563,7 @@ t; > 209: box.error.SESSION_SETTING_INVALID_VALUE > 210: box.error.SQL_PREPARE > 211: box.error.WRONG_QUERY_ID > + 212: box.error.SQL_CANT_ADD_COLUMN_TO_VIEW > ... > test_run:cmd("setopt delimiter ''"); > --- > diff --git a/test/sql-tap/alter.test.lua b/test/sql-tap/alter.test.lua > index 615b9d8a6..283dc1759 100755 > --- a/test/sql-tap/alter.test.lua > +++ b/test/sql-tap/alter.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test =3D require("sqltester") > -test:plan(50) > +test:plan(57) >=20 > test:do_execsql_test( > "alter-1.1", > @@ -585,4 +585,78 @@ test:do_catchsql_test( > -- -- > -- }) >=20 > +-- > +-- gh-3075: Check statement. > +-- > +test:do_execsql_test( > + "alter-9.1", > + [[ > + CREATE TABLE test (a INTEGER PRIMARY KEY); > + ALTER TABLE test ADD b INTEGER; > + ]], { > + -- > + -- > + }) > + > +test:do_catchsql_test( > + "alter-9.2", > + [[ > + CREATE VIEW v AS SELECT * FROM test; > + ALTER TABLE v ADD b INTEGER; > + ]], { > + -- > + 1,"Can't add column 'B' to the view 'V'" > + -- > + }) > + > +test:do_execsql_test( > + "alter-9.3", > + [[ > + ALTER TABLE test ADD c TEXT NOT NULL DEFAULT ('a') COLLATE = "unicode_ci"; > + ]], { > + -- > + -- > + }) > + > +test:do_execsql_test( > + "alter-9.4", > + [[ > + INSERT INTO test(a, b) VALUES (1, 1); > + SELECT * FROM test; > + ]], { > + -- > + 1,1,"a" > + -- > + }) > + > +test:do_catchsql_test( > + "alter-9.5", > + [[ > + INSERT INTO test VALUES (2, 2, NULL); > + ]], { > + -- > + 1,"Failed to execute SQL statement: NOT NULL constraint = failed: TEST.C" > + -- > + }) > + > +test:do_execsql_test( > + "alter-9.6", > + [[ > + SELECT * FROM test WHERE c LIKE 'A'; > + ]], { > + -- > + 1,1,"a" > + -- > + }) > + > +test:do_catchsql_test( > + "alter-9.7", > + [[ > + ALTER TABLE test ADD d INTEGER; > + ]], { > + -- > + 1,"Tuple field count 3 does not match space field count 4" > + -- > + }) > + > test:finish_test() > --=20 > 2.21.0 (Apple Git-122) >=20