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 9311846970E for ; Fri, 31 Jan 2020 18:05:41 +0300 (MSK) From: Roman Khabibov Date: Fri, 31 Jan 2020 18:05:37 +0300 Message-Id: <20200131150537.90826-1-roman.habibov@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [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 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)