From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp62.i.mail.ru (smtp62.i.mail.ru [217.69.128.42]) (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 883DA445320 for ; Mon, 6 Jul 2020 16:37:22 +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: Date: Mon, 6 Jul 2020 16:37:21 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <5CF72787-A1F0-4C48-BA8F-08F02B6960F6@tarantool.org> References: <20200403152752.8923-1-roman.habibov@tarantool.org> <20200403152752.8923-3-roman.habibov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 2/2] sql: support column addition List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review. > On Apr 25, 2020, at 01:56, Vladislav Shpilevoy = wrote: >=20 > Thanks for the patch! >=20 > You didn't respond to my 18 review comments to the previous > version of the patch. Please, do that so as I would know what > was decided on each of them. >=20 > See 3 comments below. >=20 > On 03/04/2020 17:27, Roman Khabibov wrote: >> Enable to add column to existing space with >> statement. Column definition can be >> supplemented with the four types of constraints, , >> clauses and <[NOT] NULL>, AUTOINCREMENT. >>=20 >> Closes #2349, #3075 >>=20 >> @TarantoolBot document >> Title: Add columns to existing tables in SQL >> Now, it is possible to add columns to existing empty spaces using >> >=20 > 1. Why COLUMN keyword is optional? Is it allowed by the standard? COLUMN is optional by the standard. >> statement. The column definition is the same as in >> statement, except that table constraints (PRIMARY KEY, UNIQUE, >> REFERENCES, CHECK) cannot be specified yet. >=20 > 2. Issue 2349 is about ability to add a UNIQUE column. Here you said > you 'Closes' it, and yet it can't be done, according to what you say > above. Why? Removed. > I tried that example: >=20 > tarantool> box.execute("ALTER TABLE te9 ADD s2 INT UNIQUE;") > --- > - row_count: 0 > ... >=20 > So it is not forbidden. >=20 > Also I tried to follow the template and add explicit COLUMN: >=20 > tarantool> box.execute("ALTER TABLE te9 ADD COLUMN s2 INT UNIQUE;") > --- > - null > - 'At line 1 at or near position 21: keyword ''COLUMN'' is = reserved. Please use double > quotes if ''COLUMN'' is an identifier.' > ... >=20 > Why doesn't it work? >=20 >>=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" >> ;]]) >=20 > 3. This example does not work. Gives the same error as the example = above. My bad. There was something wrong with the first patch. Now, is working. >> --- >> - row_count: 0 >> ... >=20 > I will continue the review once the comments above and the old 18 = comments > are resolved commit 82c448dc66f6233faeb40dda353652c2fd5a3d29 Author: Roman Khabibov Date: Thu Jan 2 19:06:14 2020 +0300 sql: support column addition =20 Enable to add column to existing space with statement. Column definition can be supplemented with the four types of constraints, , clauses and <[NOT] NULL>, AUTOINCREMENT. =20 Closes #2349, #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. =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 ... --- diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c index 486b6b30d..dea047241 100644 --- a/extra/mkkeywordhash.c +++ b/extra/mkkeywordhash.c @@ -76,7 +76,7 @@ static Keyword aKeywordTable[] =3D { { "CHECK", "TK_CHECK", true }, { "COLLATE", "TK_COLLATE", true }, { "COLUMN_NAME", "TK_COLUMN_NAME", 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 d1e4d02a9..3e94bee7a 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -266,6 +266,8 @@ struct errcode_record { /*211 */_(ER_WRONG_QUERY_ID, "Prepared statement with = id %u does not exist") \ /*212 */_(ER_SEQUENCE_NOT_STARTED, "Sequence '%s' = is not started") \ /*213 */_(ER_NO_SUCH_SESSION_SETTING, "Session setting %s = doesn't exist") \ + /*214 */_(ER_SQL_CANT_ADD_COLUMN_TO_VIEW, "Can't add = column '%s'. '%s' is a view") \ + /*215 */_(ER_SQL_CANT_ADD_AUTOINC, "Can't add = AUTOINCREMENT: the space '%s' already has one auto-incremented field") \ =20 /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/schema_def.h b/src/box/schema_def.h index f86cd42f1..f2af9e23f 100644 --- a/src/box/schema_def.h +++ b/src/box/schema_def.h @@ -131,6 +131,7 @@ enum { BOX_SPACE_FIELD_FIELD_COUNT =3D 4, BOX_SPACE_FIELD_OPTS =3D 5, BOX_SPACE_FIELD_FORMAT =3D 6, + box_space_field_MAX =3D 7 }; =20 /** _index fields. */ diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c index 14f6c1a97..e2bf2b7e9 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) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index ac42fe842..45fb90d38 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -285,48 +285,112 @@ sql_field_retrieve(Parse *parser, struct = space_def *space_def, uint32_t id) return field; } =20 -/* - * Add a new column to the table currently being constructed. +/** + * Make shallow copy of @a space on region. * - * The parser calls this routine once for each column declaration - * in a CREATE TABLE statement. sqlStartTable() gets called - * first to get things going. Then this routine is called for each - * column. + * Function is used to add a new column to the existing space with + * . Copy info about indexes and + * definition to create constraints appeared in the statement. */ +static struct space * +sql_shallow_space_copy(struct Parse *parse, struct space *space) +{ + assert(space->def !=3D NULL); + struct space *ret =3D sql_ephemeral_space_new(parse, = space->def->name); + if (ret =3D=3D NULL) + return NULL; + ret->index_count =3D space->index_count; + ret->index_id_max =3D space->index_id_max; + uint32_t indexes_sz =3D sizeof(struct index *) * = (ret->index_count); + ret->index =3D (struct index **) malloc(indexes_sz); + if (ret->index =3D=3D NULL) { + diag_set(OutOfMemory, indexes_sz, "realloc", = "ret->index"); + return NULL; + } + for (uint32_t i =3D 0; i < ret->index_count; i++) + ret->index[i] =3D space->index[i]; + memcpy(ret->def, space->def, sizeof(struct space_def)); + ret->def->opts.is_temporary =3D true; + ret->def->opts.is_ephemeral =3D true; + uint32_t fields_size =3D sizeof(struct field_def) * = ret->def->field_count; + ret->def->fields =3D region_alloc(&parse->region, fields_size); + if (ret->def->fields =3D=3D NULL) { + diag_set(OutOfMemory, fields_size, "region_alloc", + "ret->def->fields"); + return NULL; + } + memcpy(ret->def->fields, space->def->fields, fields_size); + + return ret; +} + void -sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def) +sql_create_column_start(struct Parse *parse) { - assert(type_def !=3D NULL); - char *z; - sql *db =3D pParse->db; - if (pParse->create_table_def.new_space =3D=3D NULL) - return; - struct space_def *def =3D = pParse->create_table_def.new_space->def; + 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); + struct space *space =3D parse->create_table_def.new_space; + bool is_alter =3D space =3D=3D NULL; + struct sql *db =3D parse->db; + if (is_alter) { + const char *space_name =3D + alter_entity_def->entity_name->a[0].zName; + space =3D space_by_name(space_name); + if (space =3D=3D NULL) { + diag_set(ClientError, ER_NO_SUCH_SPACE, = space_name); + goto tnt_error; + } + space =3D sql_shallow_space_copy(parse, space); + if (space =3D=3D NULL) + goto tnt_error; + } + create_column_def->space =3D space; + struct space_def *def =3D space->def; + assert(def->opts.is_ephemeral); =20 #if SQL_MAX_COLUMN if ((int)def->field_count + 1 > db->aLimit[SQL_LIMIT_COLUMN]) { diag_set(ClientError, ER_SQL_COLUMN_COUNT_MAX, = def->name, def->field_count + 1, = db->aLimit[SQL_LIMIT_COLUMN]); - pParse->is_aborted =3D true; - return; + goto tnt_error; } #endif + + struct region *region =3D &parse->region; + struct Token *name =3D &create_column_def->base.name; + char *column_name =3D + sql_normalized_name_region_new(region, name->z, = name->n); + if (column_name =3D=3D NULL) + goto tnt_error; + + if (parse->create_table_def.new_space =3D=3D NULL && = def->opts.is_view) { + diag_set(ClientError, ER_SQL_CANT_ADD_COLUMN_TO_VIEW, + column_name, def->name); + goto tnt_error; + } + /* - * As sql_field_retrieve will allocate memory on region - * ensure that def is also temporal and would be dropped. + * Format can be set in Lua, then exact_field_count can be + * zero, but field_count is not. */ - assert(def->opts.is_ephemeral); - if (sql_field_retrieve(pParse, def, def->field_count) =3D=3D = NULL) - return; - struct region *region =3D &pParse->region; - z =3D sql_normalized_name_region_new(region, pName->z, = pName->n); - if (z =3D=3D NULL) { - pParse->is_aborted =3D true; + if (def->exact_field_count =3D=3D 0) + def->exact_field_count =3D def->field_count; + if (sql_field_retrieve(parse, def, def->field_count) =3D=3D = NULL) return; + + for (uint32_t i =3D 0; i < def->field_count; i++) { + if (strcmp(column_name, def->fields[i].name) =3D=3D 0) { + diag_set(ClientError, = ER_SPACE_FIELD_IS_DUPLICATE, + column_name); + goto tnt_error; + } } struct field_def *column_def =3D &def->fields[def->field_count]; - memcpy(column_def, &field_def_default, = sizeof(field_def_default)); - column_def->name =3D z; + memcpy(column_def, &field_def_default, sizeof(struct = field_def)); + column_def->name =3D column_name; /* * Marker ON_CONFLICT_ACTION_DEFAULT is used to detect * attempts to define NULL multiple time or to detect @@ -334,19 +398,100 @@ sqlAddColumn(Parse * pParse, Token * pName, = struct type_def *type_def) */ column_def->nullable_action =3D ON_CONFLICT_ACTION_DEFAULT; column_def->is_nullable =3D true; - column_def->type =3D type_def->type; + column_def->type =3D create_column_def->type_def->type; def->field_count++; + + sqlSrcListDelete(db, alter_entity_def->entity_name); + return; +tnt_error: + parse->is_aborted =3D true; + sqlSrcListDelete(db, alter_entity_def->entity_name); +} + +static void +sql_vdbe_create_constraints(struct Parse *parse, int reg_space_id); + +void +sql_create_column_end(struct Parse *parse) +{ + struct create_column_def *create_column_def =3D = &parse->create_column_def; + struct space *space =3D parse->create_table_def.new_space; + bool is_alter =3D space =3D=3D NULL; + space =3D create_column_def->space; + struct space_def *def =3D space->def; + if (is_alter) { + struct field_def *field =3D = &def->fields[def->field_count - 1]; + if (field->nullable_action =3D=3D = ON_CONFLICT_ACTION_DEFAULT) { + if (create_column_def->is_pk) { + field->nullable_action =3D + ON_CONFLICT_ACTION_ABORT; + field->is_nullable =3D false; + } else { + field->nullable_action =3D + ON_CONFLICT_ACTION_NONE; + field->is_nullable =3D true; + } + } + /* + * Encode the format array and emit code to update = _space. + */ + uint32_t table_stmt_sz =3D 0; + struct region *region =3D &parse->region; + char *table_stmt =3D sql_encode_table(region, def, + &table_stmt_sz); + char *raw =3D sqlDbMallocRaw(parse->db, table_stmt_sz); + if (table_stmt =3D=3D NULL || raw =3D=3D NULL) { + parse->is_aborted =3D true; + return; + } + memcpy(raw, table_stmt, table_stmt_sz); + + struct Vdbe *v =3D sqlGetVdbe(parse); + assert(v !=3D NULL); + + struct space *system_space =3D = space_by_id(BOX_SPACE_ID); + assert(system_space !=3D NULL); + int cursor =3D parse->nTab++; + vdbe_emit_open_cursor(parse, cursor, 0, system_space); + sqlVdbeChangeP5(v, OPFLAG_SYSTEMSP); + + int key_reg =3D ++parse->nMem; + sqlVdbeAddOp2(v, OP_Integer, 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); + + int tuple_reg =3D sqlGetTempRange(parse, = box_space_field_MAX + 1); + for (int i =3D 0; i < box_space_field_MAX - 1; ++i) + sqlVdbeAddOp3(v, OP_Column, cursor, i, tuple_reg = + i); + sqlVdbeAddOp1(v, OP_Close, cursor); + + sqlVdbeAddOp2(v, OP_Integer, def->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, = box_space_field_MAX, + tuple_reg + box_space_field_MAX); + sqlVdbeAddOp4(v, OP_IdxReplace, tuple_reg + = box_space_field_MAX, + 0, 0, (char *) system_space, P4_SPACEPTR); + sql_vdbe_create_constraints(parse, key_reg); + } + + memset(create_column_def, 0, sizeof(struct create_column_def)); + create_column_def->nullable_action =3D = ON_CONFLICT_ACTION_DEFAULT; } =20 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; + struct space *space =3D parser->create_column_def.space; + assert (space !=3D NULL); + def =3D space->def; + if (NEVER(def->field_count < 1)) return; - struct space_def *def =3D space->def; - struct field_def *field =3D &def->fields[def->field_count - 1]; + 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. */ @@ -364,51 +509,46 @@ 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 a + * 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; + struct space *space =3D pParse->create_column_def.space; + assert (space !=3D NULL); + def =3D space->def; + field =3D &def->fields[def->field_count - 1]; + 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'; + goto add_default_value_exit; } + 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 @@ -447,6 +587,8 @@ sqlAddPrimaryKey(struct Parse *pParse) int nTerm; struct ExprList *pList =3D pParse->create_index_def.cols; struct space *space =3D pParse->create_table_def.new_space; + if (space =3D=3D NULL) + space =3D pParse->create_column_def.space; if (space =3D=3D NULL) goto primary_key_exit; if (sql_space_primary_key(space) !=3D NULL) { @@ -574,8 +716,10 @@ sql_create_check_contraint(struct Parse *parser) (struct alter_entity_def *) create_ck_def; assert(alter_def->entity_type =3D=3D ENTITY_TYPE_CK); (void) alter_def; - struct space *space =3D parser->create_table_def.new_space; - bool is_alter =3D space =3D=3D NULL; + struct space *space =3D parser->create_column_def.space; + if (space =3D=3D NULL) + space =3D parser->create_table_def.new_space; + bool is_alter_add_constr =3D space =3D=3D NULL; =20 /* Prepare payload for ck constraint definition. */ struct region *region =3D &parser->region; @@ -589,9 +733,18 @@ sql_create_check_contraint(struct Parse *parser) return; } } else { - assert(! is_alter); - uint32_t ck_idx =3D = ++parser->create_table_def.check_count; - name =3D tt_sprintf("ck_unnamed_%s_%d", = space->def->name, ck_idx); + assert(!is_alter_add_constr); + uint32_t idx =3D ++parser->check_count; + if (parser->create_table_def.new_space =3D=3D NULL) { + struct space *original_space =3D + space_by_name(space->def->name); + assert(original_space !=3D NULL); + struct rlist *checks =3D = &original_space->ck_constraint; + struct ck_constraint *ck; + rlist_foreach_entry(ck, checks, link) + idx++; + } + name =3D tt_sprintf("ck_unnamed_%s_%d", = space->def->name, idx); } size_t name_len =3D strlen(name); =20 @@ -634,9 +787,9 @@ sql_create_check_contraint(struct Parse *parser) trim_space_snprintf(ck_def->expr_str, expr_str, expr_str_len); memcpy(ck_def->name, name, name_len); ck_def->name[name_len] =3D '\0'; - if (is_alter) { + if (is_alter_add_constr) { const char *space_name =3D = alter_def->entity_name->a[0].zName; - struct space *space =3D space_by_name(space_name); + space =3D space_by_name(space_name); if (space =3D=3D NULL) { diag_set(ClientError, ER_NO_SUCH_SPACE, = space_name); parser->is_aborted =3D true; @@ -652,8 +805,7 @@ sql_create_check_contraint(struct Parse *parser) sqlVdbeCountChanges(v); sqlVdbeChangeP5(v, OPFLAG_NCHANGE); } else { - rlist_add_entry(&parser->create_table_def.new_check, = ck_parse, - link); + rlist_add_entry(&parser->checks, ck_parse, link); } } =20 @@ -664,18 +816,19 @@ sql_create_check_contraint(struct Parse *parser) void sqlAddCollateType(Parse * pParse, Token * pToken) { - struct space *space =3D pParse->create_table_def.new_space; - if (space =3D=3D NULL) - return; + struct space *space =3D pParse->create_column_def.space; + uint32_t *coll_id =3D NULL; + assert(space !=3D NULL); uint32_t i =3D space->def->field_count - 1; + coll_id =3D &space->def->fields[i].coll_id; 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. @@ -692,6 +845,7 @@ sqlAddCollateType(Parse * pParse, Token * pToken) sqlDbFree(db, coll_name); } =20 + struct coll * sql_column_collation(struct space_def *def, uint32_t column, uint32_t = *coll_id) { @@ -705,8 +859,7 @@ sql_column_collation(struct space_def *def, uint32_t = column, uint32_t *coll_id) * * In cases mentioned above collation is fetched by id. */ - if (space =3D=3D NULL) { - assert(def->opts.is_ephemeral); + if (def->opts.is_ephemeral) { assert(column < (uint32_t)def->field_count); *coll_id =3D def->fields[column].coll_id; struct coll_id *collation =3D coll_by_id(*coll_id); @@ -795,7 +948,8 @@ vdbe_emit_create_index(struct Parse *parse, struct = space_def *def, memcpy(raw, index_parts, index_parts_sz); index_parts =3D raw; =20 - if (parse->create_table_def.new_space !=3D NULL) { + if (parse->create_table_def.new_space !=3D NULL || + parse->create_column_def.space !=3D NULL) { sqlVdbeAddOp2(v, OP_SCopy, space_id_reg, entry_reg); sqlVdbeAddOp2(v, OP_Integer, idx_def->iid, entry_reg + = 1); } else { @@ -930,7 +1084,7 @@ emitNewSysSequenceRecord(Parse *pParse, int = reg_seq_id, const char *seq_name) static int emitNewSysSpaceSequenceRecord(Parse *pParse, int reg_space_id, int = reg_seq_id) { - uint32_t fieldno =3D pParse->create_table_def.autoinc_fieldno; + uint32_t fieldno =3D pParse->autoinc_fieldno; =20 Vdbe *v =3D sqlGetVdbe(pParse); int first_col =3D pParse->nMem + 1; @@ -1033,18 +1187,21 @@ vdbe_emit_fk_constraint_create(struct Parse = *parse_context, P4_DYNAMIC); /* * In case we are adding FK constraints during execution - * of statement, we don't have child - * id, but we know register where it will be stored. + * of or + * statement, we don't have child id, but we know register + * where it will be stored. */ - if (parse_context->create_table_def.new_space !=3D NULL) { + bool is_alter_add_constr =3D + parse_context->create_table_def.new_space =3D=3D NULL && + parse_context->create_column_def.space =3D=3D NULL; + if (!is_alter_add_constr) { sqlVdbeAddOp2(vdbe, OP_SCopy, fk->child_id, constr_tuple_reg + 1); } else { sqlVdbeAddOp2(vdbe, OP_Integer, fk->child_id, constr_tuple_reg + 1); } - if (parse_context->create_table_def.new_space !=3D NULL && - fk_constraint_is_self_referenced(fk)) { + if (!is_alter_add_constr && = fk_constraint_is_self_referenced(fk)) { sqlVdbeAddOp2(vdbe, OP_SCopy, fk->parent_id, constr_tuple_reg + 2); } else { @@ -1108,7 +1265,7 @@ vdbe_emit_fk_constraint_create(struct Parse = *parse_context, constr_tuple_reg + 9); sqlVdbeAddOp2(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID, constr_tuple_reg + 9); - if (parse_context->create_table_def.new_space =3D=3D NULL) { + if (is_alter_add_constr) { sqlVdbeCountChanges(vdbe); sqlVdbeChangeP5(vdbe, OPFLAG_NCHANGE); } @@ -1147,6 +1304,105 @@ resolve_link(struct Parse *parse_context, const = struct space_def *def, return -1; } =20 +/** + * Emit code to create sequences, indexes, check and foreign + * constraints appeared in or + * statement. + */ +static void +sql_vdbe_create_constraints(struct Parse *parse, int reg_space_id) +{ + assert(reg_space_id !=3D 0); + struct space *space =3D parse->create_table_def.new_space; + bool is_alter =3D space =3D=3D NULL; + uint32_t i =3D 0; + if (is_alter) { + space =3D parse->create_column_def.space; + i =3D space_by_name(space->def->name)->index_count; + } + assert(space !=3D NULL); + for (; i < space->index_count; ++i) { + struct index *idx =3D space->index[i]; + vdbe_emit_create_index(parse, space->def, idx->def, + reg_space_id, idx->def->iid); + } + + /* + * Check to see if we need to create an _sequence table + * for keeping track of autoincrement keys. + */ + if (parse->has_autoinc) { + /* Do an insertion into _sequence. */ + int reg_seq_id =3D ++parse->nMem; + struct Vdbe *v =3D sqlGetVdbe(parse); + assert(v !=3D NULL); + sqlVdbeAddOp2(v, OP_NextSequenceId, 0, reg_seq_id); + int reg_seq_record =3D + emitNewSysSequenceRecord(parse, reg_seq_id, + space->def->name); + const char *error_msg =3D + = tt_sprintf(tnt_errcode_desc(ER_SQL_CANT_ADD_AUTOINC), + space->def->name); + if (vdbe_emit_halt_with_presence_test(parse, + BOX_SEQUENCE_ID, = 2, + reg_seq_record + = 3, 1, + = ER_SQL_CANT_ADD_AUTOINC, + error_msg, false, + OP_NoConflict) !=3D = 0) + return; + sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, = reg_seq_record); + /* Do an insertion into _space_sequence. */ + int reg_space_seq_record =3D + emitNewSysSpaceSequenceRecord(parse, = reg_space_id, + reg_seq_id); + sqlVdbeAddOp2(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID, + reg_space_seq_record); + } + + /* Code creation of FK constraints, if any. */ + struct fk_constraint_parse *fk_parse; + rlist_foreach_entry(fk_parse, &parse->fkeys, link) { + struct fk_constraint_def *fk_def =3D fk_parse->fk_def; + if (fk_parse->selfref_cols !=3D NULL) { + struct ExprList *cols =3D = fk_parse->selfref_cols; + for (uint32_t i =3D 0; i < fk_def->field_count; = ++i) { + if (resolve_link(parse, space->def, + cols->a[i].zName, + = &fk_def->links[i].parent_field, + fk_def->name) !=3D 0) + return; + } + fk_def->parent_id =3D reg_space_id; + } else if (fk_parse->is_self_referenced) { + struct key_def *pk_key_def =3D + = sql_space_primary_key(space)->def->key_def; + if (pk_key_def->part_count !=3D = fk_def->field_count) { + diag_set(ClientError, = ER_CREATE_FK_CONSTRAINT, + fk_def->name, "number of = columns in "\ + "foreign key does not match the = "\ + "number of columns in the = primary "\ + "index of referenced table"); + parse->is_aborted =3D true; + return; + } + for (uint32_t i =3D 0; i < fk_def->field_count; = ++i) { + fk_def->links[i].parent_field =3D + pk_key_def->parts[i].fieldno; + } + fk_def->parent_id =3D reg_space_id; + } + fk_def->child_id =3D reg_space_id; + vdbe_emit_fk_constraint_create(parse, fk_def, = space->def->name); + } + + /* Code creation of CK constraints, if any. */ + struct ck_constraint_parse *ck_parse; + rlist_foreach_entry(ck_parse, &parse->checks, link) { + vdbe_emit_ck_constraint_create(parse, ck_parse->ck_def, + reg_space_id, = space->def->name); + } +} + /* * This routine is called to report the final ")" that terminates * a CREATE TABLE statement. @@ -1213,73 +1469,7 @@ sqlEndTable(struct Parse *pParse) =20 int reg_space_id =3D getNewSpaceId(pParse); vdbe_emit_space_create(pParse, reg_space_id, name_reg, = new_space); - for (uint32_t i =3D 0; i < new_space->index_count; ++i) { - struct index *idx =3D new_space->index[i]; - vdbe_emit_create_index(pParse, new_space->def, idx->def, - reg_space_id, idx->def->iid); - } - - /* - * Check to see if we need to create an _sequence table - * for keeping track of autoincrement keys. - */ - if (pParse->create_table_def.has_autoinc) { - assert(reg_space_id !=3D 0); - /* Do an insertion into _sequence. */ - int reg_seq_id =3D ++pParse->nMem; - sqlVdbeAddOp2(v, OP_NextSequenceId, 0, reg_seq_id); - int reg_seq_record =3D - emitNewSysSequenceRecord(pParse, reg_seq_id, - new_space->def->name); - sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, = reg_seq_record); - /* Do an insertion into _space_sequence. */ - int reg_space_seq_record =3D - emitNewSysSpaceSequenceRecord(pParse, = reg_space_id, - reg_seq_id); - sqlVdbeAddOp2(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID, - reg_space_seq_record); - } - /* Code creation of FK constraints, if any. */ - struct fk_constraint_parse *fk_parse; - rlist_foreach_entry(fk_parse, = &pParse->create_table_def.new_fkey, - link) { - struct fk_constraint_def *fk_def =3D fk_parse->fk_def; - if (fk_parse->selfref_cols !=3D NULL) { - struct ExprList *cols =3D = fk_parse->selfref_cols; - for (uint32_t i =3D 0; i < fk_def->field_count; = ++i) { - if (resolve_link(pParse, new_space->def, - cols->a[i].zName, - = &fk_def->links[i].parent_field, - fk_def->name) !=3D 0) - return; - } - fk_def->parent_id =3D reg_space_id; - } else if (fk_parse->is_self_referenced) { - struct index *pk =3D = sql_space_primary_key(new_space); - if (pk->def->key_def->part_count !=3D = fk_def->field_count) { - diag_set(ClientError, = ER_CREATE_FK_CONSTRAINT, - fk_def->name, "number of = columns in "\ - "foreign key does not match the = "\ - "number of columns in the = primary "\ - "index of referenced table"); - pParse->is_aborted =3D true; - return; - } - for (uint32_t i =3D 0; i < fk_def->field_count; = ++i) { - fk_def->links[i].parent_field =3D - = pk->def->key_def->parts[i].fieldno; - } - fk_def->parent_id =3D reg_space_id; - } - fk_def->child_id =3D reg_space_id; - vdbe_emit_fk_constraint_create(pParse, fk_def, = space_name_copy); - } - struct ck_constraint_parse *ck_parse; - rlist_foreach_entry(ck_parse, = &pParse->create_table_def.new_check, - link) { - vdbe_emit_ck_constraint_create(pParse, ck_parse->ck_def, - reg_space_id, = space_name_copy); - } + sql_vdbe_create_constraints(pParse, reg_space_id); } =20 void @@ -1858,24 +2048,28 @@ sql_create_foreign_key(struct Parse = *parse_context) char *parent_name =3D NULL; char *constraint_name =3D NULL; bool is_self_referenced =3D false; + struct space *space =3D parse_context->create_column_def.space; struct create_table_def *table_def =3D = &parse_context->create_table_def; - struct space *space =3D table_def->new_space; + if (space =3D=3D NULL) + space =3D table_def->new_space; /* - * Space under construction during CREATE TABLE - * processing. NULL for ALTER TABLE statement handling. + * Space under construction during + * processing or shallow copy of space during . NULL for statement handling. */ - bool is_alter =3D space =3D=3D NULL; + bool is_alter_add_constr =3D space =3D=3D NULL; uint32_t child_cols_count; struct ExprList *child_cols =3D create_fk_def->child_cols; if (child_cols =3D=3D NULL) { - assert(!is_alter); + assert(!is_alter_add_constr); child_cols_count =3D 1; } else { child_cols_count =3D child_cols->nExpr; } struct ExprList *parent_cols =3D create_fk_def->parent_cols; struct space *child_space =3D NULL; - if (is_alter) { + if (is_alter_add_constr) { const char *child_name =3D = alter_def->entity_name->a[0].zName; child_space =3D space_by_name(child_name); if (child_space =3D=3D NULL) { @@ -1893,7 +2087,9 @@ sql_create_foreign_key(struct Parse = *parse_context) goto tnt_error; } memset(fk_parse, 0, sizeof(*fk_parse)); - rlist_add_entry(&table_def->new_fkey, fk_parse, link); + if (parse_context->create_column_def.space !=3D NULL) + child_space =3D space; + rlist_add_entry(&parse_context->fkeys, fk_parse, link); } struct Token *parent =3D create_fk_def->parent_name; assert(parent !=3D NULL); @@ -1905,28 +2101,39 @@ sql_create_foreign_key(struct Parse = *parse_context) * self-referenced, but in this case parent (which is * also child) table will definitely exist. */ - is_self_referenced =3D !is_alter && + is_self_referenced =3D !is_alter_add_constr && strcmp(parent_name, space->def->name) =3D=3D = 0; struct space *parent_space =3D space_by_name(parent_name); - if (parent_space =3D=3D NULL) { - if (is_self_referenced) { - struct fk_constraint_parse *fk =3D - rlist_first_entry(&table_def->new_fkey, - struct = fk_constraint_parse, - link); - fk->selfref_cols =3D parent_cols; - fk->is_self_referenced =3D true; - } else { - diag_set(ClientError, ER_NO_SUCH_SPACE, = parent_name);; - goto tnt_error; - } + if (parent_space =3D=3D NULL && !is_self_referenced) { + diag_set(ClientError, ER_NO_SUCH_SPACE, parent_name);; + goto tnt_error; + } + if (is_self_referenced) { + struct fk_constraint_parse *fk =3D + rlist_first_entry(&parse_context->fkeys, + struct fk_constraint_parse, + link); + fk->selfref_cols =3D parent_cols; + fk->is_self_referenced =3D true; } - if (!is_alter) { + if (!is_alter_add_constr) { if (create_def->name.n =3D=3D 0) { - constraint_name =3D - sqlMPrintf(db, "fk_unnamed_%s_%d", - space->def->name, - ++table_def->fkey_count); + uint32_t idx =3D ++parse_context->fkey_count; + if (table_def->new_space =3D=3D NULL) { + struct space *original_space =3D + space_by_name(space->def->name); + assert(original_space !=3D NULL); + struct rlist *child_fk =3D + = &original_space->child_fk_constraint; + if (!rlist_empty(child_fk)) { + struct fk_constraint *fk; + rlist_foreach_entry(fk, = child_fk, + = in_child_space) + idx++; + } + } + constraint_name =3D sqlMPrintf(db, = "fk_unnamed_%s_%d", + space->def->name, = idx); } else { constraint_name =3D sql_name_from_token(db, = &create_def->name); @@ -1986,7 +2193,8 @@ sql_create_foreign_key(struct Parse = *parse_context) } int actions =3D create_fk_def->actions; fk_def->field_count =3D child_cols_count; - fk_def->child_id =3D child_space !=3D NULL ? = child_space->def->id : 0; + fk_def->child_id =3D table_def->new_space =3D=3D NULL ? + child_space->def->id : 0; fk_def->parent_id =3D parent_space !=3D NULL ? = parent_space->def->id : 0; fk_def->is_deferred =3D create_constr_def->is_deferred; fk_def->match =3D (enum fk_constraint_match) = (create_fk_def->match); @@ -2006,7 +2214,7 @@ sql_create_foreign_key(struct Parse = *parse_context) constraint_name) !=3D 0) { goto exit_create_fk; } - if (!is_alter) { + if (!is_alter_add_constr) { if (child_cols =3D=3D NULL) { assert(i =3D=3D 0); /* @@ -2035,14 +2243,15 @@ sql_create_foreign_key(struct Parse = *parse_context) memcpy(fk_def->name, constraint_name, name_len); fk_def->name[name_len] =3D '\0'; /* - * In case of CREATE TABLE processing, all foreign keys - * constraints must be created after space itself, so - * lets delay it until sqlEndTable() call and simply + * In case of or processing, all foreign keys constraints must + * be created after space itself, so lets delay it until + * sqlEndTable() or sql_add_column_end() call and simply * maintain list of all FK constraints inside parser. */ - if (!is_alter) { + if (!is_alter_add_constr) { struct fk_constraint_parse *fk_parse =3D - rlist_first_entry(&table_def->new_fkey, + rlist_first_entry(&parse_context->fkeys, struct fk_constraint_parse, = link); fk_parse->fk_def =3D fk_def; } else { @@ -2065,12 +2274,10 @@ tnt_error: void fk_constraint_change_defer_mode(struct Parse *parse_context, bool = is_deferred) { - if (parse_context->db->init.busy || - rlist_empty(&parse_context->create_table_def.new_fkey)) + if (parse_context->db->init.busy || = rlist_empty(&parse_context->fkeys)) return; - rlist_first_entry(&parse_context->create_table_def.new_fkey, - struct fk_constraint_parse, = link)->fk_def->is_deferred =3D - is_deferred; + rlist_first_entry(&parse_context->fkeys, struct = fk_constraint_parse, + link)->fk_def->is_deferred =3D is_deferred; } =20 /** @@ -2394,7 +2601,10 @@ sql_create_index(struct Parse *parse) { * Find the table that is to be indexed. * Return early if not found. */ - struct space *space =3D NULL; + struct space *space =3D parse->create_table_def.new_space; + if (space =3D=3D NULL) + space =3D parse->create_column_def.space; + bool is_create_table_or_add_col =3D space !=3D NULL; struct Token token =3D create_entity_def->name; if (tbl_name !=3D NULL) { assert(token.n > 0 && token.z !=3D NULL); @@ -2407,10 +2617,8 @@ sql_create_index(struct Parse *parse) { } goto exit_create_index; } - } else { - if (parse->create_table_def.new_space =3D=3D NULL) - goto exit_create_index; - space =3D parse->create_table_def.new_space; + } else if (space =3D=3D NULL) { + goto exit_create_index; } struct space_def *def =3D space->def; =20 @@ -2445,7 +2653,7 @@ sql_create_index(struct Parse *parse) { * 2) UNIQUE constraint is non-named and standard * auto-index name will be generated. */ - if (parse->create_table_def.new_space =3D=3D NULL) { + if (!is_create_table_or_add_col) { assert(token.z !=3D NULL); name =3D sql_name_from_token(db, &token); if (name =3D=3D NULL) { @@ -2611,7 +2819,7 @@ sql_create_index(struct Parse *parse) { * constraint, but has different onError (behavior on * constraint violation), then an error is raised. */ - if (parse->create_table_def.new_space !=3D NULL) { + if (is_create_table_or_add_col) { for (uint32_t i =3D 0; i < space->index_count; ++i) { struct index *existing_idx =3D space->index[i]; uint32_t iid =3D existing_idx->def->iid; @@ -2699,7 +2907,7 @@ sql_create_index(struct Parse *parse) { sqlVdbeAddOp0(vdbe, OP_Expire); } =20 - if (tbl_name !=3D NULL) + if (!is_create_table_or_add_col) goto exit_create_index; table_add_index(space, index); index =3D NULL; @@ -3306,15 +3514,15 @@ vdbe_emit_halt_with_presence_test(struct Parse = *parser, int space_id, int sql_add_autoincrement(struct Parse *parse_context, uint32_t fieldno) { - if (parse_context->create_table_def.has_autoinc) { + if (parse_context->has_autoinc) { diag_set(ClientError, ER_SQL_SYNTAX_WITH_POS, parse_context->line_count, = parse_context->line_pos, "table must feature at most one AUTOINCREMENT = field"); parse_context->is_aborted =3D true; return -1; } - parse_context->create_table_def.has_autoinc =3D true; - parse_context->create_table_def.autoinc_fieldno =3D fieldno; + parse_context->has_autoinc =3D true; + parse_context->autoinc_fieldno =3D fieldno; return 0; } =20 diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 995875566..5904414a3 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -226,19 +226,23 @@ create_table_end ::=3D . { sqlEndTable(pParse); } */ =20 columnlist ::=3D columnlist COMMA tcons. -columnlist ::=3D columnlist COMMA columnname carglist autoinc(I). { - uint32_t fieldno =3D = pParse->create_table_def.new_space->def->field_count - 1; - if (I =3D=3D 1 && sql_add_autoincrement(pParse, fieldno) !=3D 0) - return; +columnlist ::=3D columnlist COMMA column_def create_column_end. +columnlist ::=3D column_def create_column_end. +columnlist ::=3D tcons. + +column_def ::=3D column_name_and_type carglist. + +column_name_and_type ::=3D nm(A) typedef(Y). { + create_column_def_init(&pParse->create_column_def, NULL, &A, &Y); + sql_create_column_start(pParse); } =20 -columnlist ::=3D columnname carglist autoinc(I). { - uint32_t fieldno =3D = pParse->create_table_def.new_space->def->field_count - 1; +create_column_end ::=3D autoinc(I). { + uint32_t fieldno =3D = pParse->create_column_def.space->def->field_count - 1; if (I =3D=3D 1 && sql_add_autoincrement(pParse, fieldno) !=3D 0) return; + sql_create_column_end(pParse); } -columnlist ::=3D tcons. -columnname(A) ::=3D nm(A) typedef(Y). {sqlAddColumn(pParse,&A,&Y);} =20 // An IDENTIFIER can be a generic identifier, or one of several // keywords. Any non-standard keyword can also be an identifier. @@ -281,9 +285,11 @@ nm(A) ::=3D id(A). { } } =20 -// "carglist" is a list of additional constraints that come after the -// column name and column type in a CREATE TABLE statement. -// +/* + * "carglist" is a list of additional constraints and clauses that + * come after the column name and column type in a + * or statement. + */ carglist ::=3D carglist ccons. carglist ::=3D . %type cconsname { struct Token } @@ -314,6 +320,7 @@ 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); sqlAddPrimaryKey(pParse); + pParse->create_column_def.is_pk =3D true; } ccons ::=3D cconsname(N) UNIQUE. { create_index_def_init(&pParse->create_index_def, NULL, &N, NULL, @@ -1735,11 +1742,28 @@ 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 COLUMN nm(A). { N =3D A; } +column_name(N) ::=3D nm(A). { N =3D A; } + +cmd ::=3D alter_column_def carglist create_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_create_column_start(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 cb0ecd2fc..91f9affa2 100644 --- a/src/box/sql/parse_def.h +++ b/src/box/sql/parse_def.h @@ -35,6 +35,7 @@ #include "box/fk_constraint.h" #include "box/key_def.h" #include "box/sql.h" +#include "box/constraint_id.h" =20 /** * This file contains auxiliary structures and functions which @@ -154,6 +155,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, @@ -205,26 +207,22 @@ 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_column_def { + struct create_entity_def base; + /** Shallow space_def copy. */ + struct space *space; + /* True if this column has constraint. */ + bool is_pk; + /** Column type. */ + struct type_def *type_def; + /** Token from clause. */ + struct Token *collate; + /** Action to perform if NULL constraint failed. */ + enum on_conflict_action nullable_action; + /** String from clause. */ + struct ExprSpan *default_clause; }; =20 struct create_view_def { @@ -482,9 +480,17 @@ create_table_def_init(struct create_table_def = *table_def, struct Token *name, { create_entity_def_init(&table_def->base, ENTITY_TYPE_TABLE, = NULL, name, if_not_exists); - rlist_create(&table_def->new_fkey); - rlist_create(&table_def->new_check); - table_def->autoinc_fieldno =3D 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 =3D type_def; + column_def->is_pk =3D false; } =20 static inline void @@ -499,14 +505,4 @@ create_view_def_init(struct create_view_def = *view_def, struct Token *name, view_def->aliases =3D aliases; } =20 -static inline void -create_table_def_destroy(struct create_table_def *table_def) -{ - if (table_def->new_space =3D=3D NULL) - return; - struct fk_constraint_parse *fk; - rlist_foreach_entry(fk, &table_def->new_fkey, link) - sql_expr_list_delete(sql_get(), fk->selfref_cols); -} - #endif /* TARANTOOL_BOX_SQL_PARSE_DEF_H_INCLUDED */ diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c index a5a258805..b31bac437 100644 --- a/src/box/sql/prepare.c +++ b/src/box/sql/prepare.c @@ -197,9 +197,13 @@ sql_parser_create(struct Parse *parser, struct sql = *db, uint32_t sql_flags) { memset(parser, 0, sizeof(struct Parse)); parser->db =3D db; + parser->create_column_def.nullable_action =3D = ON_CONFLICT_ACTION_DEFAULT; parser->sql_flags =3D sql_flags; parser->line_count =3D 1; parser->line_pos =3D 1; + rlist_create(&parser->fkeys); + rlist_create(&parser->checks); + parser->has_autoinc =3D false; region_create(&parser->region, &cord()->slabc); } =20 @@ -211,7 +215,9 @@ sql_parser_destroy(Parse *parser) sql *db =3D parser->db; sqlDbFree(db, parser->aLabel); sql_expr_list_delete(db, parser->pConstExpr); - create_table_def_destroy(&parser->create_table_def); + struct fk_constraint_parse *fk; + rlist_foreach_entry(fk, &parser->fkeys, link) + sql_expr_list_delete(sql_get(), fk->selfref_cols); if (db !=3D NULL) { assert(db->lookaside.bDisable >=3D parser->disableLookaside); diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index aa6a470f8..3143ec521 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -2249,12 +2249,26 @@ struct Parse { struct enable_entity_def enable_entity_def; }; /** - * Table def is not part of union since information - * being held must survive till the end of parsing of - * whole CREATE TABLE statement (to pass it to - * sqlEndTable() function). + * Table def or column def is not part of union since + * information being held must survive till the end of + * parsing of whole or + * statement (to pass it to + * sqlEndTable() sql_create_column_end() function). */ struct create_table_def create_table_def; + struct create_column_def create_column_def; + /** + * FK and CK constraints appeared in a or + * a statement. + */ + struct rlist fkeys; + struct rlist checks; + uint32_t fkey_count; + uint32_t check_count; + /** True, if column to be created has . */ + bool has_autoinc; + /** Id of field with . */ + uint32_t autoinc_fieldno; bool initiateTTrans; /* Initiate Tarantool transaction */ /** If set - do not emit byte code at all, just parse. */ bool parse_only; @@ -2844,15 +2858,31 @@ struct space *sqlResultSetOfSelect(Parse *, = Select *); =20 struct space * sqlStartTable(Parse *, Token *); -void sqlAddColumn(Parse *, Token *, struct type_def *); + +/** + * Add new field to the format of ephemeral space in + * create_table_def. If it is create shallow copy of + * the existing space and add field to its format. + */ +void +sql_create_column_start(struct Parse *parse); + +/** + * Nullify create_column_def. If it is emit code to + * update entry in _space and code to create constraints (entries + * in _index, _ck_constraint, _fk_constraint) described with this + * column. + */ +void +sql_create_column_end(struct Parse *parse); =20 /** * This routine is called by the parser while in the middle of - * parsing a CREATE TABLE statement. A "NOT NULL" constraint has - * been seen on a column. This routine sets the is_nullable flag - * on the column currently under construction. - * If nullable_action has been already set, this function raises - * an error. + * parsing a or a + * statement. A "NOT NULL" constraint has been seen on a column. + * This routine sets the is_nullable flag on the column currently + * under construction. If nullable_action has been already set, + * this function raises an error. * * @param parser SQL Parser object. * @param nullable_action on_conflict_action value. diff --git a/test/box/error.result b/test/box/error.result index 2196fa541..eb55f9cdf 100644 --- a/test/box/error.result +++ b/test/box/error.result @@ -432,6 +432,8 @@ t; | 211: box.error.WRONG_QUERY_ID | 212: box.error.SEQUENCE_NOT_STARTED | 213: box.error.NO_SUCH_SESSION_SETTING + | 214: box.error.SQL_CANT_ADD_COLUMN_TO_VIEW + | 215: box.error.SQL_CANT_ADD_AUTOINC | ... =20 test_run:cmd("setopt delimiter ''"); diff --git a/test/sql/add-column.result b/test/sql/add-column.result new file mode 100644 index 000000000..06c95facb --- /dev/null +++ b/test/sql/add-column.result @@ -0,0 +1,276 @@ +-- test-run result file version 2 +test_run =3D require('test_run').new() + | --- + | ... +engine =3D test_run:get_cfg('engine') + | --- + | ... +_ =3D box.space._session_settings:update('sql_default_engine', {{'=3D', = 2, engine}}) + | --- + | ... + +-- +-- gh-3075: Check statement. +-- +box.execute('CREATE TABLE t1 (a INTEGER PRIMARY KEY);') + | --- + | - row_count: 1 + | ... +-- +-- COLUMN keyword is optional. Check it here, but omit it below. +-- +box.execute('ALTER TABLE t1 ADD COLUMN b INTEGER;') + | --- + | - row_count: 0 + | ... + +-- +-- Can't add column to a view. +-- +box.execute('CREATE VIEW v AS SELECT * FROM t1;') + | --- + | - row_count: 1 + | ... +box.execute('ALTER TABLE v ADD b INTEGER;') + | --- + | - null + | - Can't add column 'B'. 'V' is a view + | ... +box.execute('DROP VIEW v;') + | --- + | - row_count: 1 + | ... + +-- +-- Check column constraints typing and work. +-- +box.execute('CREATE TABLE t2 (a INTEGER CONSTRAINT pk_constr PRIMARY = KEY);') + | --- + | - row_count: 1 + | ... +box.execute('ALTER TABLE t2 DROP CONSTRAINT pk_constr') + | --- + | - row_count: 1 + | ... +test_run:cmd("setopt delimiter ';'"); + | --- + | - true + | ... +box.execute([[ALTER TABLE t2 ADD b INTEGER CONSTRAINT pk_constr PRIMARY = KEY + CHECK (b > 0) + REFERENCES t1(a) + CONSTRAINT u_constr UNIQUE]]) +test_run:cmd("setopt delimiter ''"); + | --- + | ... +box.execute('INSERT INTO t1 VALUES (1, 1);') + | --- + | - row_count: 1 + | ... +box.execute('INSERT INTO t2 VALUES (1, 1);') + | --- + | - row_count: 1 + | ... +box.execute('INSERT INTO t2 VALUES (1, 1);') + | --- + | - null + | - Duplicate key exists in unique index 'PK_CONSTR' in space 'T2' + | ... + +box.execute('INSERT INTO t1 VALUES (0, 1);') + | --- + | - row_count: 1 + | ... +box.execute('INSERT INTO t2 VALUES (2, 0);') + | --- + | - null + | - 'Check constraint failed ''ck_unnamed_T2_1'': b > 0' + | ... + +box.execute('INSERT INTO t2 VALUES (2, 3);') + | --- + | - null + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | ... + +box.execute('DROP TABLE t2;') + | --- + | - row_count: 1 + | ... + +-- +-- Check self-referenced FK creation. +-- +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY);') + | --- + | - row_count: 1 + | ... +box.execute('ALTER TABLE t2 ADD b INT REFERENCES t1') + | --- + | - row_count: 0 + | ... + +box.execute('DROP TABLE t2;') + | --- + | - row_count: 1 + | ... + +-- +-- Check AUTOINCREMENT work. +-- +box.execute("CREATE TABLE t2(a INTEGER CONSTRAINT pk PRIMARY KEY);") + | --- + | - row_count: 1 + | ... +box.execute("ALTER TABLE t2 DROP CONSTRAINT pk;") + | --- + | - row_count: 1 + | ... +box.execute("ALTER TABLE t2 ADD b INTEGER PRIMARY KEY AUTOINCREMENT;") + | --- + | - row_count: 0 + | ... +box.execute("ALTER TABLE t2 ADD c INTEGER AUTOINCREMENT;") + | --- + | - null + | - 'Can''t add AUTOINCREMENT: the space ''T2'' already has one = auto-incremented field' + | ... + +box.execute('DROP TABLE t2;') + | --- + | - row_count: 1 + | ... + +-- +-- Check clauses after column typing and work. +-- +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY, b INTEGER);') + | --- + | - row_count: 1 + | ... +test_run:cmd("setopt delimiter ';'"); + | --- + | - true + | ... +box.execute([[ALTER TABLE t2 ADD c TEXT NOT NULL DEFAULT ('a') + COLLATE "unicode_ci";]]); + | --- + | - row_count: 0 + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... +box.execute('INSERT INTO t2(a, b) VALUES (1, 1);') + | --- + | - row_count: 1 + | ... +box.execute('SELECT * FROM t2;') + | --- + | - metadata: + | - name: A + | type: integer + | - name: B + | type: integer + | - name: C + | type: string + | rows: + | - [1, 1, 'a'] + | ... +box.execute('INSERT INTO t2 VALUES (2, 2, NULL);') + | --- + | - null + | - 'Failed to execute SQL statement: NOT NULL constraint failed: = T2.C' + | ... +box.execute('SELECT * FROM t2 WHERE c LIKE \'A\';') + | --- + | - metadata: + | - name: A + | type: integer + | - name: B + | type: integer + | - name: C + | type: string + | rows: + | - [1, 1, 'a'] + | ... + +-- +-- Try to add to a non-empty space a [non-]nullable field. +-- +box.execute('ALTER TABLE t2 ADD d INTEGER;') + | --- + | - null + | - Tuple field count 3 does not match space field count 4 + | ... +box.execute('ALTER TABLE t2 ADD d TEXT NOT NULL'); + | --- + | - null + | - Tuple field count 3 does not match space field count 4 + | ... +box.execute('ALTER TABLE t2 ADD e TEXT NULL'); + | --- + | - null + | - Tuple field count 3 does not match space field count 4 + | ... + +-- +-- Add to a space with no-SQL adjusted or without format. +-- +_ =3D box.schema.space.create('WITHOUT') + | --- + | ... +box.execute("ALTER TABLE WITHOUT ADD a INTEGER;") + | --- + | - row_count: 0 + | ... +box.execute("DROP TABLE WITHOUT;") + | --- + | - row_count: 1 + | ... + +s =3D box.schema.space.create('NOSQL') + | --- + | ... +s:format{{name =3D 'A', type =3D 'unsigned'}} + | --- + | ... +box.execute("ALTER TABLE NOSQL ADD b INTEGER") + | --- + | - row_count: 0 + | ... + +box.execute('DROP TABLE t2;') + | --- + | - row_count: 1 + | ... +-- +-- Add multiple columns inside a transaction. +-- +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY)') + | --- + | - row_count: 1 + | ... +box.begin() = \ +box.execute('ALTER TABLE t2 ADD b INT') = \ +box.execute('ALTER TABLE t2 ADD c INT') = \ +box.commit() + | --- + | ... + +box.execute('INSERT INTO t2 VALUES (1, 1, 1)') + | --- + | - row_count: 1 + | ... +box.execute('SELECT * FROM t2;') + | --- + | - metadata: + | - name: A + | type: integer + | - name: B + | type: integer + | - name: C + | type: integer + | rows: + | - [1, 1, 1] + | ... diff --git a/test/sql/add-column.test.lua b/test/sql/add-column.test.lua new file mode 100644 index 000000000..f0ce1c44c --- /dev/null +++ b/test/sql/add-column.test.lua @@ -0,0 +1,103 @@ +test_run =3D require('test_run').new() +engine =3D test_run:get_cfg('engine') +_ =3D box.space._session_settings:update('sql_default_engine', {{'=3D', = 2, engine}}) + +-- +-- gh-3075: Check statement. +-- +box.execute('CREATE TABLE t1 (a INTEGER PRIMARY KEY);') +-- +-- COLUMN keyword is optional. Check it here, but omit it below. +-- +box.execute('ALTER TABLE t1 ADD COLUMN b INTEGER;') + +-- +-- Can't add column to a view. +-- +box.execute('CREATE VIEW v AS SELECT * FROM t1;') +box.execute('ALTER TABLE v ADD b INTEGER;') +box.execute('DROP VIEW v;') + +-- +-- Check column constraints typing and work. +-- +box.execute('CREATE TABLE t2 (a INTEGER CONSTRAINT pk_constr PRIMARY = KEY);') +box.execute('ALTER TABLE t2 DROP CONSTRAINT pk_constr') +test_run:cmd("setopt delimiter ';'"); +box.execute([[ALTER TABLE t2 ADD b INTEGER CONSTRAINT pk_constr PRIMARY = KEY + CHECK (b > 0) + REFERENCES t1(a) + CONSTRAINT u_constr UNIQUE]]) +test_run:cmd("setopt delimiter ''"); +box.execute('INSERT INTO t1 VALUES (1, 1);') +box.execute('INSERT INTO t2 VALUES (1, 1);') +box.execute('INSERT INTO t2 VALUES (1, 1);') + +box.execute('INSERT INTO t1 VALUES (0, 1);') +box.execute('INSERT INTO t2 VALUES (2, 0);') + +box.execute('INSERT INTO t2 VALUES (2, 3);') + +box.execute('DROP TABLE t2;') + +-- +-- Check self-referenced FK creation. +-- +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY);') +box.execute('ALTER TABLE t2 ADD b INT REFERENCES t1') + +box.execute('DROP TABLE t2;') + +-- +-- Check AUTOINCREMENT work. +-- +box.execute("CREATE TABLE t2(a INTEGER CONSTRAINT pk PRIMARY KEY);") +box.execute("ALTER TABLE t2 DROP CONSTRAINT pk;") +box.execute("ALTER TABLE t2 ADD b INTEGER PRIMARY KEY AUTOINCREMENT;") +box.execute("ALTER TABLE t2 ADD c INTEGER AUTOINCREMENT;") + +box.execute('DROP TABLE t2;') + +-- +-- Check clauses after column typing and work. +-- +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY, b INTEGER);') +test_run:cmd("setopt delimiter ';'"); +box.execute([[ALTER TABLE t2 ADD c TEXT NOT NULL DEFAULT ('a') + COLLATE "unicode_ci";]]); +test_run:cmd("setopt delimiter ''"); +box.execute('INSERT INTO t2(a, b) VALUES (1, 1);') +box.execute('SELECT * FROM t2;') +box.execute('INSERT INTO t2 VALUES (2, 2, NULL);') +box.execute('SELECT * FROM t2 WHERE c LIKE \'A\';') + +-- +-- Try to add to a non-empty space a [non-]nullable field. +-- +box.execute('ALTER TABLE t2 ADD d INTEGER;') +box.execute('ALTER TABLE t2 ADD d TEXT NOT NULL'); +box.execute('ALTER TABLE t2 ADD e TEXT NULL'); + +-- +-- Add to a space with no-SQL adjusted or without format. +-- +_ =3D box.schema.space.create('WITHOUT') +box.execute("ALTER TABLE WITHOUT ADD a INTEGER;") +box.execute("DROP TABLE WITHOUT;") + +s =3D box.schema.space.create('NOSQL') +s:format{{name =3D 'A', type =3D 'unsigned'}} +box.execute("ALTER TABLE NOSQL ADD b INTEGER") + +box.execute('DROP TABLE t2;') +-- +-- Add multiple columns inside a transaction. +-- +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY)') +box.begin() = \ +box.execute('ALTER TABLE t2 ADD b INT') = \ +box.execute('ALTER TABLE t2 ADD c INT') = \ +box.commit() + +box.execute('INSERT INTO t2 VALUES (1, 1, 1)') +box.execute('SELECT * FROM t2;')