From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 A5B57469719 for ; Tue, 10 Mar 2020 01:16:36 +0300 (MSK) References: <20200131150537.90826-1-roman.habibov@tarantool.org> <7F7A0DA7-C5F2-43FD-AFF4-F6F4C700D603@tarantool.org> From: Vladislav Shpilevoy Message-ID: <61fa3221-1be2-0895-311a-88e9e151360a@tarantool.org> Date: Mon, 9 Mar 2020 23:16:34 +0100 MIME-Version: 1.0 In-Reply-To: <7F7A0DA7-C5F2-43FD-AFF4-F6F4C700D603@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Roman Khabibov , tarantool-patches@dev.tarantool.org Hi! Sure. On 09/03/2020 16:08, Roman Khabibov wrote: > Vlad, can you take it, please? > >> On Jan 31, 2020, at 18:05, Roman Khabibov wrote: >> >> Enable to add column to existing space with >> statement. Column definition can be >> supplemented with , clauses and <[NOT] NULL> >> column constraint. >> >> Closes #3075 >> >> @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. >> >> For example: >> >> tarantool> box.execute([[CREATE TABLE test ( >> a INTEGER PRIMARY KEY >> );]]) >> --- >> - row_count: 1 >> ... >> >> 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-column >> Issue: https://github.com/tarantool/tarantool/issues/3075 >> >> 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: >> >> 1) CREATE TABLE (parse->create_table_def.new_space != 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. >> >> 2) ALTER TABLE (parse->create_table_def.new_space == NULL) >> We work with existing space = space_by_name(), but don't modify it. >> This space will be modified after parsing by alter.cc triggers. >> >> 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. >> >> There is obvious way to implement constraints parsing during >> ADD COLUMN. To do partial copy of space = 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. >> >> I would like to hear your opinion. Maybe I'm generally wrong in >> thinking about all this, and it could be made easier. >> >> 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) ::= COLUMN nm(A). { N = A; } >> because I had: >> /Users/r.khabibov/tarantool/src/box/sql/parse.h:171:9: error: 'TK_COLUMN' macro redefined >> #define TK_COLUMN >> >> 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. >> >> { "COLUMN", "TK_COLUMN", true }, >> { "CONSTRAINT", "TK_CONSTRAINT", true }, >> >> 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(-) >> >> 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[] = { >> { "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'") \ >> >> /* >> * !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) >> */ >> >> 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 = 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); >> >> - assert(def != NULL); >> - uint32_t field_count = def->field_count; >> + assert(fields != NULL); >> mpstream_encode_array(&stream, field_count); >> for (uint32_t i = 0; i < field_count && !is_error; i++) { >> - uint32_t cid = def->fields[i].coll_id; >> - struct field_def *field = &def->fields[i]; >> + struct field_def *field = &fields[i]; >> + uint32_t cid = field->coll_id; >> const char *default_str = field->default_value; >> int base_len = 4; >> if (cid != 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 == >> - action_is_nullable(def->fields[i].nullable_action)); >> + assert(field->is_nullable == >> + 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"); >> >> - assert(def->fields[i].nullable_action < on_conflict_action_MAX); >> + assert(field->nullable_action < on_conflict_action_MAX); >> const char *action = >> - on_conflict_action_strs[def->fields[i].nullable_action]; >> + on_conflict_action_strs[field->nullable_action]; >> mpstream_encode_str(&stream, action); >> if (cid != 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; >> } >> >> +char * >> +sql_encode_table(struct region *region, struct space_def *def, uint32_t *size) >> +{ >> + assert(def != 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" >> >> 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 = &parse->create_column_def; >> + struct alter_entity_def *alter_entity_def = >> + &create_column_def->base.base; >> + assert(alter_entity_def->entity_type == ENTITY_TYPE_COLUMN); >> + assert(alter_entity_def->alter_action == ALTER_ACTION_CREATE); >> + >> + const char *space_name = >> + alter_entity_def->entity_name->a[0].zName; >> + struct space *space = space_by_name(space_name); >> + struct sql *db = parse->db; >> + if (space == NULL) { >> + diag_set(ClientError, ER_NO_SUCH_SPACE, space_name); >> + goto tnt_error; >> + } >> + >> + struct space_def *def = space->def; >> + create_column_def->space_def = def; >> + if (sql_space_def_check_format(def) != 0) >> + goto tnt_error; >> + uint32_t new_field_count = def->field_count + 1; >> + create_column_def->new_field_count = 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 = sizeof(struct field_def) * new_field_count; >> + struct region *region = &parse->region; >> + struct field_def *new_fields = region_alloc(region, size); >> + if (new_fields == 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 = new_fields; >> + struct field_def *new_column_def = &new_fields[def->field_count]; >> + >> + memcpy(new_column_def, &field_def_default, sizeof(struct field_def)); >> + struct Token *name = &create_column_def->base.name; >> + new_column_def->name = >> + sql_normalized_name_region_new(region, name->z, name->n); >> + if (new_column_def->name == 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 = create_column_def->type_def->type; >> + >> +exit_alter_add_column_start: >> + sqlSrcListDelete(db, alter_entity_def->entity_name); >> + return; >> +tnt_error: >> + parse->is_aborted = true; >> + goto exit_alter_add_column_start; >> +} >> + >> +void >> +sql_alter_add_column_end(struct Parse *parse) >> +{ >> + struct create_column_def *create_column_def = &parse->create_column_def; >> + uint32_t new_field_count = create_column_def->new_field_count; >> + uint32_t table_stmt_sz = 0; >> + char *table_stmt = >> + sql_encode_table_format(&parse->region, >> + create_column_def->new_fields, >> + new_field_count, &table_stmt_sz); >> + if (table_stmt == NULL) { >> + parse->is_aborted = true; >> + return; >> + } >> + char *raw = sqlDbMallocRaw(parse->db, table_stmt_sz); >> + if (raw == NULL){ >> + parse->is_aborted = true; >> + return; >> + } >> + memcpy(raw, table_stmt, table_stmt_sz); >> + >> + struct space *system_space = space_by_id(BOX_SPACE_ID); >> + assert(system_space != NULL); >> + int cursor = parse->nTab++; >> + struct Vdbe *v = sqlGetVdbe(parse); >> + assert(v != NULL); >> + vdbe_emit_open_cursor(parse, cursor, 0, system_space); >> + sqlVdbeChangeP5(v, OPFLAG_SYSTEMSP); >> + >> + int key_reg = sqlGetTempReg(parse); >> + sqlVdbeAddOp2(v, OP_Integer, create_column_def->space_def->id, key_reg); >> + int addr = 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 = 7; >> + int tuple_reg = sqlGetTempRange(parse, system_space_field_count + 1); >> + for (int i = 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 = parser->create_table_def.new_space; >> - if (space == NULL || NEVER(space->def->field_count < 1)) >> + struct space_def *def = NULL; >> + struct field_def *field = NULL; >> + if (parser->create_table_def.new_space != NULL) { >> + def = parser->create_table_def.new_space->def; >> + if (NEVER(def->field_count < 1)) >> + return; >> + field = &def->fields[def->field_count - 1]; >> + } else if (parser->create_column_def.space_def != NULL) { >> + def = parser->create_column_def.space_def; >> + uint32_t field_count = >> + parser->create_column_def.new_field_count; >> + field = &parser->create_column_def.new_fields[field_count - 1]; >> + } else { >> return; >> - struct space_def *def = space->def; >> - struct field_def *field = &def->fields[def->field_count - 1]; >> + } >> if (field->nullable_action != ON_CONFLICT_ACTION_DEFAULT && >> nullable_action != field->nullable_action) { >> /* Prevent defining nullable_action many times. */ >> @@ -390,51 +400,53 @@ sql_column_add_nullable_action(struct Parse *parser, >> } >> >> /* >> - * 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 = pParse->db; >> - struct space *p = pParse->create_table_def.new_space; >> - if (p != NULL) { >> - assert(p->def->opts.is_ephemeral); >> - struct space_def *def = p->def; >> - if (!sqlExprIsConstantOrFunction >> - (pSpan->pExpr, db->init.busy)) { >> - const char *column_name = >> - 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 = NULL; >> + struct field_def *field = NULL; >> + if (pParse->create_table_def.new_space != NULL) { >> + def = pParse->create_table_def.new_space->def; >> + assert(def->opts.is_ephemeral); >> + field = &def->fields[def->field_count - 1]; >> + } else if (pParse->create_column_def.space_def != NULL) { >> + def = pParse->create_column_def.space_def; >> + uint32_t field_count = >> + pParse->create_column_def.new_field_count; >> + field = &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 = true; >> + } else { >> + assert(def != NULL); >> + struct region *region = &pParse->region; >> + uint32_t default_length = (int)(pSpan->zEnd - pSpan->zStart); >> + field->default_value = region_alloc(region, default_length + 1); >> + if (field->default_value == NULL) { >> + diag_set(OutOfMemory, default_length + 1, >> + "region_alloc", "field->default_value"); >> pParse->is_aborted = true; >> - } else { >> - assert(def != NULL); >> - struct field_def *field = >> - &def->fields[def->field_count - 1]; >> - struct region *region = &pParse->region; >> - uint32_t default_length = (int)(pSpan->zEnd - pSpan->zStart); >> - field->default_value = region_alloc(region, >> - default_length + 1); >> - if (field->default_value == NULL) { >> - diag_set(OutOfMemory, default_length + 1, >> - "region_alloc", >> - "field->default_value"); >> - pParse->is_aborted = true; >> - return; >> - } >> - strncpy(field->default_value, pSpan->zStart, >> - default_length); >> - field->default_value[default_length] = '\0'; >> + return; >> } >> + strncpy(field->default_value, pSpan->zStart, default_length); >> + field->default_value[default_length] = '\0'; >> } >> +add_default_value_exit: >> sql_expr_delete(db, pSpan->pExpr, false); >> } >> >> @@ -686,17 +698,27 @@ void >> sqlAddCollateType(Parse * pParse, Token * pToken) >> { >> struct space *space = pParse->create_table_def.new_space; >> - if (space == NULL) >> + uint32_t *coll_id = NULL; >> + if (space != NULL) { >> + uint32_t i = space->def->field_count - 1; >> + coll_id = &space->def->fields[i].coll_id; >> + } else if (pParse->create_column_def.space_def != NULL) { >> + uint32_t field_count = >> + pParse->create_column_def.new_field_count; >> + struct field_def *field = >> + &pParse->create_column_def.new_fields[field_count - 1]; >> + coll_id = &field->coll_id; >> + } else { >> return; >> - uint32_t i = space->def->field_count - 1; >> + } >> sql *db = pParse->db; >> char *coll_name = sql_name_from_token(db, pToken); >> if (coll_name == NULL) { >> pParse->is_aborted = true; >> return; >> } >> - uint32_t *coll_id = &space->def->fields[i].coll_id; >> - if (sql_get_coll_seq(pParse, coll_name, coll_id) != NULL) { >> + if (sql_get_coll_seq(pParse, coll_name, coll_id) != NULL && >> + space != 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 == NULL) >> goto error; >> uint32_t table_stmt_sz = 0; >> - char *table_stmt = sql_encode_table(region, space->def, &table_stmt_sz); >> + char *table_stmt = sql_encode_table_format(region, space->def->fields, >> + space->def->field_count, >> + &table_stmt_sz); >> if (table_stmt == NULL) >> goto error; >> char *raw = 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 ::= . >> %type cconsname { struct Token } >> cconsname(N) ::= CONSTRAINT nm(X). { N = X; } >> cconsname(N) ::= . { N = Token_nil; } >> -ccons ::= DEFAULT term(X). {sqlAddDefaultValue(pParse,&X);} >> -ccons ::= DEFAULT LP expr(X) RP. {sqlAddDefaultValue(pParse,&X);} >> -ccons ::= DEFAULT PLUS term(X). {sqlAddDefaultValue(pParse,&X);} >> -ccons ::= DEFAULT MINUS(A) term(X). { >> +ccons ::= default_clause_1. >> +default_clause_1 ::= DEFAULT term(X). {sqlAddDefaultValue(pParse,&X);} >> +ccons ::= default_clause_2. >> +default_clause_2 ::= DEFAULT LP expr(X) RP. {sqlAddDefaultValue(pParse,&X);} >> +ccons ::= default_clause_3. >> +default_clause_3 ::= DEFAULT PLUS term(X). {sqlAddDefaultValue(pParse,&X);} >> +ccons ::= default_clause_4. >> +default_clause_4 ::= DEFAULT MINUS(A) term(X). { >> ExprSpan v; >> v.pExpr = sqlPExpr(pParse, TK_UMINUS, X.pExpr, 0); >> v.zStart = A.z; >> @@ -303,13 +307,18 @@ ccons ::= DEFAULT MINUS(A) term(X). { >> // In addition to the type name, we also care about the primary key and >> // UNIQUE constraints. >> // >> -ccons ::= NULL onconf(R). { >> +ccons ::= null_cons. >> +null_cons ::= NULL onconf(R). { >> sql_column_add_nullable_action(pParse, ON_CONFLICT_ACTION_NONE); >> /* Trigger nullability mismatch error if required. */ >> if (R != ON_CONFLICT_ACTION_ABORT) >> sql_column_add_nullable_action(pParse, R); >> } >> -ccons ::= NOT NULL onconf(R). {sql_column_add_nullable_action(pParse, R);} >> +ccons ::= not_null_cons. >> +not_null_cons ::= NOT NULL onconf(R). { >> + sql_column_add_nullable_action(pParse, R); >> +} >> + >> ccons ::= 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 ::= cconsname(N) REFERENCES nm(T) eidlist_opt(TA) matcharg(M) refargs(R). >> sql_create_foreign_key(pParse); >> } >> ccons ::= defer_subclause(D). {fk_constraint_change_defer_mode(pParse, D);} >> -ccons ::= COLLATE id(C). {sqlAddCollateType(pParse, &C);} >> +ccons ::= collate_clause. >> +collate_clause ::= COLLATE id(C). {sqlAddCollateType(pParse, &C);} >> >> // The optional AUTOINCREMENT keyword >> %type autoinc {int} >> @@ -1729,11 +1739,46 @@ alter_table_start(A) ::= ALTER TABLE fullname(T) . { A = T; } >> >> %type alter_add_constraint {struct alter_args} >> alter_add_constraint(A) ::= alter_table_start(T) ADD CONSTRAINT nm(N). { >> + A.table_name = T; >> + A.name = N; >> + pParse->initiateTTrans = true; >> + } >> + >> +%type alter_add_column {struct alter_args} >> +alter_add_column(A) ::= alter_table_start(T) ADD column_name(N). { >> A.table_name = T; >> A.name = N; >> pParse->initiateTTrans = true; >> } >> >> +column_name(N) ::= nm(A). { N = A; } >> + >> +cmd ::= alter_column_def alter_carglist add_column_end. >> + >> +alter_column_def ::= 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 ::= alter_carglist alter_ccons. >> +alter_carglist ::= . >> +alter_ccons ::= default_clause_1. >> +alter_ccons ::= default_clause_2. >> +alter_ccons ::= default_clause_3. >> +alter_ccons ::= default_clause_4. >> +alter_ccons ::= null_cons. >> +alter_ccons ::= not_null_cons. >> +alter_ccons ::= collate_clause. >> + >> +add_column_end ::= . { >> + sql_alter_add_column_end(pParse); >> +} >> + >> cmd ::= 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 { >> >> enum entity_type { >> ENTITY_TYPE_TABLE = 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; >> }; >> >> +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 = 0; >> } >> >> +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 = 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 *); >> >> +/** >> + * 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); >> >> +/** >> + * 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 *); >> >> @@ -4031,6 +4057,20 @@ sql_alter_table_rename(struct Parse *parse); >> void >> sql_alter_ck_constraint_enable(struct Parse *parse); >> >> +/** >> + * 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); >> >> /** >> * 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 = require("sqltester") >> -test:plan(50) >> +test:plan(57) >> >> test:do_execsql_test( >> "alter-1.1", >> @@ -585,4 +585,78 @@ test:do_catchsql_test( >> -- -- >> -- }) >> >> +-- >> +-- 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() >> -- >> 2.21.0 (Apple Git-122) >> >