From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 3B8A5469719 for ; Tue, 29 Sep 2020 02:28:56 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.1\)) From: Roman Khabibov In-Reply-To: <20200918125901.GL10599@tarantool.org> Date: Tue, 29 Sep 2020 02:28:53 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <9C52EA98-F9AD-44A5-8755-E2A76372964E@tarantool.org> References: <20200911215115.6622-1-roman.habibov@tarantool.org> <20200911215115.6622-7-roman.habibov@tarantool.org> <20200916201823.GE10599@tarantool.org> <4b304f32-379a-fb50-f427-615a91646b7d@tarantool.org> <20200918125901.GL10599@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 6/6] sql: support column addition List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Hi! Thanks for the review. > On Sep 18, 2020, at 15:59, Nikita Pettik = wrote: >=20 > On 17 Sep 17:19, Vladislav Shpilevoy wrote: >> Thanks for the comments! I tried to answer some of them. >>=20 >> On 16.09.2020 22:18, Nikita Pettik wrote: >>> On 12 Sep 00:51, 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 >>> Space emptyness is required by ansi or it is tarantool's = restriction? >>=20 >> It is Tarantool's restriction. >=20 > I mean it would be nice to explain this in the doc request.. Done. >=20 >>> In Postgres for instance, one can add column for non-empty table and >>> it is considered to be ok (new column is filled with default value). >>> We can use ephemeral tables as temporary holder while rebuilding = space. >>=20 >> Ephemeral spaces can't be used at least because of vinyl, because may >> not fit into the memory. >=20 > Anyway they are used for vinyl even now. >=20 >> With Roman we decided to implement non-empty >> alter later as a separate issue. >=20 > Ok, then please create follow-up issue with proposed approach. We may patch the box code and implement DEFAULT (and NULL ?)inside box. This trigger will update space=E2=80=99s tuples during . In any case, we need to patch box. We will do it later. >>>> diff --git a/src/box/errcode.h b/src/box/errcode.h >>>> index 3c21375f5..ec16399a0 100644 >>>> --- a/src/box/errcode.h >>>> +++ b/src/box/errcode.h >>>> @@ -271,6 +271,7 @@ struct errcode_record { >>>> /*216 */_(ER_SYNC_QUORUM_TIMEOUT, "Quorum collection = for a synchronous transaction is timed out") \ >>>> /*217 */_(ER_SYNC_ROLLBACK, "A rollback for a = synchronous transaction is received") \ >>>> /*218 */_(ER_TUPLE_METADATA_IS_TOO_BIG, "Can't create tuple: = metadata size %u is too big") \ >>>> + /*219 */_(ER_SQL_CANT_ADD_AUTOINC, "Can't add = AUTOINCREMENT: the space '%s' already has one auto incremented field") \ >>>=20 >>> Consider reusing sql_add_autoincrement() func. >>=20 >> We can't. sql_add_autoincrement() fails only if autoinc is added more = than once >> inside one statements. Here Roman tries to handle an error, when = autoinc was >> added earlier in another statement, so we have has_autoinc false in = ADD COLUMN. >> Even if autoinc was specified in CREATE TABLE some time ago. >=20 > Just set parse_context->create_table_def.has_autoinc before func > invocation to space->sequence_fieldno. Unfourtrinally, it is two different errors. The error in sql_add_autoincrement() is syntax error with position, it means: "there can be no more than two AUTOINCREMENT words in CREATE = TABLE=E2=80=9D. The error in sql_vdbe_create_constraints() is a clarification of the box = error that "the key in space _sequence is duplicated". As far as I understand, = I can "raise" this check from the box level to the parser level as follows: = copy the space_object->sequence pointer to parser_ephemeral_space->sequence = inside the space_shallow_copy() function and check it for NULL when = AUTOINCREMENT is faced within a statement. But it seems to me that it is not worth doing this, = because in the future we want to get rid of the use of box objects (struct = space, struct index) during parsing? Also, I can remove check (error) in sql_add_autoincrement() for = unification. The check will take place in box only for both and , after parsing. Is it worth it? >>> Or re-phrase err msg and use it >>> in sql_add_autoinc. For example: >>> space %s can't feature more than one AUTOINCREMENT field. >>>=20 >>>> /* >>>> * !IMPORTANT! Please follow instructions at start of the file >>>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >>>> index d1d240315..677099b48 100644 >>>> --- a/src/box/sql/build.c >>>> +++ b/src/box/sql/build.c >>>> @@ -285,48 +285,110 @@ 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 an existing space with >>>> + * statement. Copy space def and index >>>> + * array to create constraints appeared in the statement. The >>>> + * index array copy will be modified by adding new elements to it. >>>> + * It is necessary, because the statement may contain several >>>> + * index definitions (constraints). >>>> */ >>>> +static struct space * >>>> +sql_shallow_space_copy(struct Parse *parse, struct space *space) >>>=20 >>> Why do you need whole space *? Def is likely to be enough. >>=20 >> We need to store indexes somewhere. UNIQUE, for example. Space_def >> does not have index definitions. >=20 > Hm, we need at most one index_def. Mb it is worth passing it > as a separate arg? There can be huge number of named UNIQUE indexes. commit 515026fac40d90596ebdeb03f2b06d39085e4830 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 * Space emptiness is Tarantool's restriction. Possibilty to add column to non empty space will be implemented later. =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 > CHECK = (LENGTH(b) > 1) > NOT NULL > DEFAULT ('aa') > COLLATE = "unicode_ci" > ]]) --- - row_count: 1 ... ``` diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c index bacdad9b3..7480c0211 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_REF", "TK_COLUMN_REF", 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 e6957d612..5a59f477f 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -273,6 +273,8 @@ struct errcode_record { /*218 */_(ER_TUPLE_METADATA_IS_TOO_BIG, "Can't create tuple: = metadata size %u is too big") \ /*219 */_(ER_XLOG_GAP, "%s") \ /*220 */_(ER_TOO_EARLY_SUBSCRIBE, "Can't subscribe = non-anonymous replica %s until join is done") \ + /*221 */_(ER_SQL_CANT_ADD_AUTOINC, "Can't add = AUTOINCREMENT: space %s can't feature more than one AUTOINCREMENT = field") \ + =20 /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/sql/build.c b/src/box/sql/build.c index d1d240315..599e9b43f 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -285,48 +285,110 @@ 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 an existing space with + * statement. Copy space def and index + * array to create constraints appeared in the statement. The + * index array copy will be modified by adding new elements to it. + * It is necessary, because the statement may contain several + * index definitions (constraints). */ +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) + goto error; + ret->index_count =3D space->index_count; + ret->index_id_max =3D space->index_id_max; + size_t size =3D 0; + ret->index =3D region_alloc_array(&parse->region, typeof(struct = index *), + ret->index_count, &size); + if (ret->index =3D=3D NULL) { + diag_set(OutOfMemory, size, "region_alloc_array", = "ret->index"); + goto error; + } + memcpy(ret->index, space->index, size); + memcpy(ret->def, space->def, sizeof(struct space_def)); + ret->def->opts.is_temporary =3D true; + ret->def->opts.is_ephemeral =3D true; + if (ret->def->field_count !=3D 0) { + uint32_t fields_size =3D 0; + ret->def->fields =3D + region_alloc_array(&parse->region, + typeof(struct field_def), + ret->def->field_count, = &fields_size); + if (ret->def->fields =3D=3D NULL) { + diag_set(OutOfMemory, fields_size, = "region_alloc", + "ret->def->fields"); + goto error; + } + memcpy(ret->def->fields, space->def->fields, = fields_size); + } + + return ret; +error: + parse->is_aborted =3D true; + return NULL; +} + 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; + /* - * 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) + 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; - 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; - return; - } + 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; + 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,18 +396,86 @@ 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 space *space =3D parse->create_column_def.space; + assert(space !=3D NULL); + struct space_def *def =3D space->def; + struct field_def *field =3D &def->fields[def->field_count - 1]; + if (field->nullable_action =3D=3D ON_CONFLICT_ACTION_DEFAULT) { + 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 + BOX_SPACE_FIELD_FIELD_COUNT); + sqlVdbeAddOp4(v, OP_Blob, table_stmt_sz, + tuple_reg + BOX_SPACE_FIELD_FORMAT, + 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); + sqlVdbeCountChanges(v); + sqlVdbeChangeP5(v, OPFLAG_NCHANGE); + sqlReleaseTempRange(parse, tuple_reg, box_space_field_MAX + 1); + sql_vdbe_create_constraints(parse, key_reg); } =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)) + assert(parser->create_column_def.space !=3D NULL); + struct space_def *def =3D parser->create_column_def.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]; if (field->nullable_action !=3D ON_CONFLICT_ACTION_DEFAULT && nullable_action !=3D field->nullable_action) { @@ -364,20 +494,21 @@ 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 an + * statement. */ void sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan) { sql *db =3D pParse->db; - struct space *p =3D pParse->create_table_def.new_space; + struct space *p =3D pParse->create_column_def.space; if (p !=3D NULL) { assert(p->def->opts.is_ephemeral); struct space_def *def =3D p->def; @@ -447,6 +578,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 +707,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,8 +724,22 @@ sql_create_check_contraint(struct Parse *parser) return; } } else { - assert(! is_alter); + assert(!is_alter_add_constr); uint32_t ck_idx =3D ++parser->checks_def.count; + /* + * If it is we should + * count the existing CHECK constraints in the + * space and form a name based on this. + */ + 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 ck_constraint *ck; + rlist_foreach_entry(ck, = &original_space->ck_constraint, + link) + ck_idx++; + } name =3D tt_sprintf("ck_unnamed_%s_%d", = space->def->name, ck_idx); } size_t name_len =3D strlen(name); @@ -634,7 +783,7 @@ 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); if (space =3D=3D NULL) { @@ -663,9 +812,8 @@ 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; + assert(space !=3D NULL); uint32_t i =3D space->def->field_count - 1; sql *db =3D pParse->db; char *coll_name =3D sql_name_from_token(db, pToken); @@ -695,7 +843,6 @@ struct coll * sql_column_collation(struct space_def *def, uint32_t column, uint32_t = *coll_id) { assert(def !=3D NULL); - struct space *space =3D space_by_id(def->id); /* * It is not always possible to fetch collation directly * from struct space due to its absence in space cache. @@ -704,13 +851,13 @@ 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); return collation !=3D NULL ? collation->coll : NULL; } + struct space *space =3D space_by_id(def->id); struct tuple_field *field =3D tuple_format_field(space->format, = column); *coll_id =3D field->coll_id; return field->coll; @@ -794,7 +941,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 { @@ -1032,18 +1180,23 @@ 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 (we know it, but + * fk->child_id stores register because of code reuse in + * sql_vdbe_create_constraints()), 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 { @@ -1107,7 +1260,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); } @@ -1148,15 +1301,28 @@ resolve_link(struct Parse *parse_context, const = struct space_def *def, =20 /** * Emit code to create sequences, indexes, check and foreign key - * constraints appeared in . + * constraints appeared in or + * . */ 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; - assert(space !=3D NULL); + bool is_alter =3D space =3D=3D NULL; uint32_t i =3D 0; + /* + * If it is an , then we have to + * create all indexes added by this statement. These + * indexes are in the array, starting with old index_count + * (inside space object) and ending with new index_count + * (inside ephemeral space). + */ + 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, @@ -1175,6 +1341,21 @@ sql_vdbe_create_constraints(struct Parse *parse, = int reg_space_id) sqlVdbeAddOp2(v, OP_NextSequenceId, 0, reg_seq_id); int reg_seq_rec =3D emitNewSysSequenceRecord(parse, = reg_seq_id, = space->def->name); + if (is_alter) { + int errcode =3D ER_SQL_CANT_ADD_AUTOINC; + const char *error_msg =3D + tt_sprintf(tnt_errcode_desc(errcode), + space->def->name); + if (vdbe_emit_halt_with_presence_test(parse, + = BOX_SEQUENCE_ID, + 2, + = reg_seq_rec + 3, + 1, = errcode, + error_msg, = false, + = OP_NoConflict) + !=3D 0) + return; + } sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, = reg_seq_rec); /* Do an insertion into _space_sequence. */ int reg_space_seq_record =3D @@ -1873,24 +2054,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) { @@ -1908,6 +2093,12 @@ sql_create_foreign_key(struct Parse = *parse_context) goto tnt_error; } memset(fk_parse, 0, sizeof(*fk_parse)); + /* + * Child space already exists if it is + * . + */ + if (table_def->new_space =3D=3D NULL) + child_space =3D space; rlist_add_entry(&parse_context->fkeys_def.fkeys, = fk_parse, link); } @@ -1921,29 +2112,42 @@ 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 rlist *fkeys =3D = &parse_context->fkeys_def.fkeys; - struct fk_constraint_parse *fk =3D - rlist_first_entry(fkeys, - 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_def.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, - = ++parse_context->fkeys_def.count); + uint32_t idx =3D = ++parse_context->fkeys_def.count; + /* + * If it is we + * should count the existing FK + * constraints in the space and form a + * name based on this. + */ + 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; + 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); @@ -2023,7 +2227,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); /* @@ -2052,12 +2256,14 @@ 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 - * maintain list of all FK constraints inside parser. + * In case of <=D0=A1REATE TABLE> and + * processing, all foreign keys constraints must be + * created after space itself (or space altering), so let + * delay it until sql_vdbe_create_constraints() 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(&parse_context->fkeys_def.fkeys, struct fk_constraint_parse, = link); @@ -2430,7 +2636,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); @@ -2443,10 +2652,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 (!is_create_table_or_add_col) { + goto exit_create_index; } struct space_def *def =3D space->def; =20 @@ -2481,7 +2688,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) { @@ -2647,7 +2854,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; @@ -2735,7 +2942,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; sql_space_add_index(&parse->region, space, index); index =3D NULL; diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index d81045d50..3c50c6bbd 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -228,19 +228,24 @@ 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. + +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; + if (pParse->create_table_def.new_space =3D=3D NULL) + 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. @@ -283,9 +288,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 } @@ -1737,11 +1744,30 @@ 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); + create_checks_def_init(&pParse->checks_def); + create_fkeys_def_init(&pParse->fkeys_def); + 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 c44e3dbbe..29bc22ea8 100644 --- a/src/box/sql/parse_def.h +++ b/src/box/sql/parse_def.h @@ -107,9 +107,10 @@ struct fk_constraint_parse { */ struct fk_constraint_def *fk_def; /** - * If inside CREATE TABLE statement we want to declare - * self-referenced FK constraint, we must delay their - * resolution until the end of parsing of all columns. + * If inside or + * statement we want to declare self-referenced FK + * constraint, we must delay their resolution until the + * end of parsing of all columns. * E.g.: CREATE TABLE t1(id REFERENCES t1(b), b); */ struct ExprList *selfref_cols; @@ -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, @@ -207,6 +209,14 @@ struct create_table_def { struct space *new_space; }; =20 +struct create_column_def { + struct create_entity_def base; + /** Shallow space copy. */ + struct space *space; + /** Column type. */ + struct type_def *type_def; +}; + struct create_checks_def { struct rlist checks; uint32_t count; @@ -474,6 +484,16 @@ create_table_def_init(struct create_table_def = *table_def, struct Token *name, if_not_exists); } =20 +static inline void +create_column_def_init(struct create_column_def *column_def, + struct SrcList *table_name, struct Token *name, + struct type_def *type_def) +{ + create_entity_def_init(&column_def->base, ENTITY_TYPE_COLUMN, + table_name, name, false); + column_def->type_def =3D type_def; +} + static inline void create_checks_def_init(struct create_checks_def *checks_def) { diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 7057c20b6..7190ef19d 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -2251,20 +2251,24 @@ 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 . + * FK and CK constraints appeared in a or + * an statement. */ struct create_fkeys_def fkeys_def; struct create_checks_def checks_def; /* - * True, if column within a statement to be - * created has . + * True, if column in a or an + * statement to be created has + * . */ bool has_autoinc; /* Id of field with . */ @@ -2858,15 +2862,30 @@ 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_column_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); + +/** + * 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/src/box/sql/tokenize.c b/src/box/sql/tokenize.c index 5404a78e9..0899cf23e 100644 --- a/src/box/sql/tokenize.c +++ b/src/box/sql/tokenize.c @@ -40,6 +40,7 @@ #include =20 #include "box/session.h" +#include "box/schema.h" #include "say.h" #include "sqlInt.h" #include "tarantoolInt.h" @@ -431,8 +432,8 @@ sql_token(const char *z, int *type, bool = *is_reserved) =20 /** * This function is called to release parsing artifacts - * during table creation. The only objects allocated using - * malloc are index defs and check constraints. + * during table creation or column addition. The only objects + * allocated using malloc are index defs. * Note that this functions can't be called on ordinary * space object. It's purpose is to clean-up parser->new_space. * @@ -445,7 +446,15 @@ parser_space_delete(struct sql *db, struct space = *space) if (space =3D=3D NULL || db =3D=3D NULL) return; assert(space->def->opts.is_ephemeral); - for (uint32_t i =3D 0; i < space->index_count; ++i) + struct space *altered_space =3D space_by_name(space->def->name); + uint32_t i =3D 0; + /* + * Don't delete already existing defs and start from new + * ones. + */ + if (altered_space !=3D NULL) + i =3D altered_space->index_count; + for (; i < space->index_count; ++i) index_def_delete(space->index[i]->def); } =20 @@ -539,7 +548,7 @@ sqlRunParser(Parse * pParse, const char *zSql) sqlVdbeDelete(pParse->pVdbe); pParse->pVdbe =3D 0; } - parser_space_delete(db, pParse->create_table_def.new_space); + parser_space_delete(db, pParse->create_column_def.space); =20 if (pParse->pWithToFree) sqlWithDelete(db, pParse->pWithToFree); diff --git a/test/box/error.result b/test/box/error.result index 4d85b9e55..2f6d7fb22 100644 --- a/test/box/error.result +++ b/test/box/error.result @@ -439,6 +439,7 @@ t; | 218: box.error.TUPLE_METADATA_IS_TOO_BIG | 219: box.error.XLOG_GAP | 220: box.error.TOO_EARLY_SUBSCRIBE + | 221: 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..924368ea2 --- /dev/null +++ b/test/sql/add-column.result @@ -0,0 +1,529 @@ +-- test-run result file version 2 +-- +-- gh-3075: Check statement. +-- +CREATE TABLE t1 (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... + +-- +-- COLUMN keyword is optional. Check it here, but omit it below. +-- +ALTER TABLE t1 ADD COLUMN b INT; + | --- + | - row_count: 1 + | ... + +-- +-- A column with the same name already exists. +-- +ALTER TABLE t1 ADD b SCALAR; + | --- + | - null + | - Space field 'B' is duplicate + | ... + +-- +-- Can't add column to a view. +-- +CREATE VIEW v AS SELECT * FROM t1; + | --- + | - row_count: 1 + | ... +ALTER TABLE v ADD c INT; + | --- + | - null + | - 'Can''t modify space ''V'': view can not be altered' + | ... + +\set language lua + | --- + | - true + | ... +view =3D box.space._space.index[2]:select('V')[1]:totable() + | --- + | ... +view_format =3D view[7] + | --- + | ... +f =3D {type =3D 'string', nullable_action =3D 'none', name =3D 'C', = is_nullable =3D true} + | --- + | ... +table.insert(view_format, f) + | --- + | ... +view[5] =3D 3 + | --- + | ... +view[7] =3D view_format + | --- + | ... +box.space._space:replace(view) + | --- + | - error: 'Can''t modify space ''V'': view can not be altered' + | ... +\set language sql + | --- + | - true + | ... + +DROP VIEW v; + | --- + | - row_count: 1 + | ... + +-- +-- Check PRIMARY KEY constraint works with an added column. +-- +CREATE TABLE pk_check (a INT CONSTRAINT pk PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE pk_check DROP CONSTRAINT pk; + | --- + | - row_count: 1 + | ... +ALTER TABLE pk_check ADD b INT PRIMARY KEY; + | --- + | - row_count: 1 + | ... +INSERT INTO pk_check VALUES (1, 1); + | --- + | - row_count: 1 + | ... +INSERT INTO pk_check VALUES (1, 1); + | --- + | - null + | - Duplicate key exists in unique index 'pk_unnamed_PK_CHECK_1' in = space 'PK_CHECK' + | ... +DROP TABLE pk_check; + | --- + | - row_count: 1 + | ... + +-- +-- Check UNIQUE constraint works with an added column. +-- +CREATE TABLE unique_check (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE unique_check ADD b INT UNIQUE; + | --- + | - row_count: 1 + | ... +INSERT INTO unique_check VALUES (1, 1); + | --- + | - row_count: 1 + | ... +INSERT INTO unique_check VALUES (2, 1); + | --- + | - null + | - Duplicate key exists in unique index = 'unique_unnamed_UNIQUE_CHECK_2' in space 'UNIQUE_CHECK' + | ... +DROP TABLE unique_check; + | --- + | - row_count: 1 + | ... + +-- +-- Check CHECK constraint works with an added column. +-- +CREATE TABLE ck_check (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE ck_check ADD b INT CHECK (b > 0); + | --- + | - row_count: 1 + | ... +INSERT INTO ck_check VALUES (1, 0); + | --- + | - null + | - 'Check constraint failed ''ck_unnamed_CK_CHECK_1'': b > 0' + | ... +INSERT INTO ck_check VALUES (1, 1); + | --- + | - row_count: 1 + | ... +DROP TABLE ck_check; + | --- + | - row_count: 1 + | ... + +-- +-- Check FOREIGN KEY constraint works with an added column. +-- +CREATE TABLE fk_check (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE fk_check ADD b INT REFERENCES t1(a); + | --- + | - row_count: 1 + | ... +INSERT INTO fk_check VALUES (0, 1); + | --- + | - null + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | ... +INSERT INTO fk_check VALUES (2, 0); + | --- + | - null + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | ... +INSERT INTO fk_check VALUES (2, 1); + | --- + | - null + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | ... +INSERT INTO t1 VALUES (1, 1); + | --- + | - row_count: 1 + | ... +INSERT INTO fk_check VALUES (2, 1); + | --- + | - row_count: 1 + | ... +DROP TABLE fk_check; + | --- + | - row_count: 1 + | ... +DROP TABLE t1; + | --- + | - row_count: 1 + | ... +-- +-- Check FOREIGN KEY (self-referenced) constraint works with an +-- added column. +-- +CREATE TABLE self (id INT PRIMARY KEY AUTOINCREMENT, a INT UNIQUE) + | --- + | - row_count: 1 + | ... +ALTER TABLE self ADD b INT REFERENCES self(a) + | --- + | - row_count: 1 + | ... +INSERT INTO self(a,b) VALUES(1, 1); + | --- + | - autoincrement_ids: + | - 1 + | row_count: 1 + | ... +UPDATE self SET a =3D 2, b =3D 2; + | --- + | - row_count: 1 + | ... +UPDATE self SET b =3D 3; + | --- + | - null + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | ... +UPDATE self SET a =3D 3; + | --- + | - null + | - 'Failed to execute SQL statement: FOREIGN KEY constraint failed' + | ... +DROP TABLE self; + | --- + | - row_count: 1 + | ... + +-- +-- Check AUTOINCREMENT works with an added column. +-- +CREATE TABLE autoinc_check (a INT CONSTRAINT pk PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE autoinc_check DROP CONSTRAINT pk; + | --- + | - row_count: 1 + | ... +ALTER TABLE autoinc_check ADD b INT PRIMARY KEY AUTOINCREMENT; + | --- + | - row_count: 1 + | ... +INSERT INTO autoinc_check(a) VALUES(1); + | --- + | - autoincrement_ids: + | - 1 + | row_count: 1 + | ... +INSERT INTO autoinc_check(a) VALUES(1); + | --- + | - autoincrement_ids: + | - 2 + | row_count: 1 + | ... +TRUNCATE TABLE autoinc_check; + | --- + | - row_count: 0 + | ... + +-- +-- Can't add second column with AUTOINCREMENT. +-- +ALTER TABLE autoinc_check ADD c INT AUTOINCREMENT; + | --- + | - null + | - 'Can''t add AUTOINCREMENT: space AUTOINC_CHECK can''t feature more = than one AUTOINCREMENT + | field' + | ... +DROP TABLE autoinc_check; + | --- + | - row_count: 1 + | ... + +-- +-- Check COLLATE clause works with an added column. +-- +CREATE TABLE collate_check (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE collate_check ADD b TEXT COLLATE "unicode_ci"; + | --- + | - row_count: 1 + | ... +INSERT INTO collate_check VALUES (1, 'a'); + | --- + | - row_count: 1 + | ... +INSERT INTO collate_check VALUES (2, 'A'); + | --- + | - row_count: 1 + | ... +SELECT * FROM collate_check WHERE b LIKE 'a'; + | --- + | - metadata: + | - name: A + | type: integer + | - name: B + | type: string + | rows: + | - [1, 'a'] + | - [2, 'A'] + | ... +DROP TABLE collate_check; + | --- + | - row_count: 1 + | ... + +-- +-- Check DEFAULT clause works with an added column. +-- +CREATE TABLE default_check (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE default_check ADD b TEXT DEFAULT ('a'); + | --- + | - row_count: 1 + | ... +INSERT INTO default_check(a) VALUES (1); + | --- + | - row_count: 1 + | ... +SELECT * FROM default_check; + | --- + | - metadata: + | - name: A + | type: integer + | - name: B + | type: string + | rows: + | - [1, 'a'] + | ... +DROP TABLE default_check; + | --- + | - row_count: 1 + | ... + +-- +-- Check NULL constraint works with an added column. +-- +CREATE TABLE null_check (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE null_check ADD b TEXT NULL; + | --- + | - row_count: 1 + | ... +INSERT INTO null_check(a) VALUES (1); + | --- + | - row_count: 1 + | ... +SELECT * FROM null_check; + | --- + | - metadata: + | - name: A + | type: integer + | - name: B + | type: string + | rows: + | - [1, null] + | ... +DROP TABLE null_check; + | --- + | - row_count: 1 + | ... + +-- +-- Check NOT NULL constraint works with an added column. +-- +CREATE TABLE notnull_check (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... +ALTER TABLE notnull_check ADD b TEXT NOT NULL; + | --- + | - row_count: 1 + | ... +INSERT INTO notnull_check(a) VALUES (1); + | --- + | - null + | - 'Failed to execute SQL statement: NOT NULL constraint failed: = NOTNULL_CHECK.B' + | ... +INSERT INTO notnull_check VALUES (1, 'not null'); + | --- + | - row_count: 1 + | ... +DROP TABLE notnull_check; + | --- + | - row_count: 1 + | ... + +-- +-- Can't add a column with DEAFULT or NULL to a non-empty space. +-- This ability isn't implemented yet. +-- +CREATE TABLE non_empty (a INT PRIMARY KEY); + | --- + | - row_count: 1 + | ... +INSERT INTO non_empty VALUES (1); + | --- + | - row_count: 1 + | ... +ALTER TABLE non_empty ADD b INT NULL; + | --- + | - null + | - Tuple field count 1 does not match space field count 2 + | ... +ALTER TABLE non_empty ADD b INT DEFAULT (1); + | --- + | - null + | - Tuple field count 1 does not match space field count 2 + | ... +DROP TABLE non_empty; + | --- + | - row_count: 1 + | ... + +-- +-- Add to a no-SQL adjusted space without format. +-- +\set language lua + | --- + | - true + | ... +_ =3D box.schema.space.create('WITHOUT_FORMAT') + | --- + | ... +\set language sql + | --- + | - true + | ... +ALTER TABLE without_format ADD a INT PRIMARY KEY; + | --- + | - row_count: 1 + | ... +INSERT INTO without_format VALUES (1); + | --- + | - row_count: 1 + | ... +DROP TABLE without_format; + | --- + | - row_count: 1 + | ... + +-- +-- Add to a no-SQL adjusted space with format. +-- +\set language lua + | --- + | - true + | ... +with_format =3D box.schema.space.create('WITH_FORMAT') + | --- + | ... +with_format:format{{name =3D 'A', type =3D 'unsigned'}} + | --- + | ... +\set language sql + | --- + | - true + | ... +ALTER TABLE with_format ADD b INT PRIMARY KEY; + | --- + | - row_count: 1 + | ... +INSERT INTO with_format VALUES (1, 1); + | --- + | - row_count: 1 + | ... +DROP TABLE with_format; + | --- + | - row_count: 1 + | ... + +-- +-- Add multiple columns (with a constraint) inside a transaction. +-- +CREATE TABLE t2 (a INT PRIMARY KEY) + | --- + | - row_count: 1 + | ... +\set language lua + | --- + | - true + | ... +box.begin() = \ +box.execute('ALTER TABLE t2 ADD b INT') = \ +box.execute('ALTER TABLE t2 ADD c INT UNIQUE') = \ +box.commit() + | --- + | ... +\set language sql + | --- + | - true + | ... +INSERT INTO t2 VALUES (1, 1, 1); + | --- + | - row_count: 1 + | ... +INSERT INTO t2 VALUES (2, 1, 1); + | --- + | - null + | - Duplicate key exists in unique index 'unique_unnamed_T2_2' in = space 'T2' + | ... +SELECT * FROM t2; + | --- + | - metadata: + | - name: A + | type: integer + | - name: B + | type: integer + | - name: C + | type: integer + | rows: + | - [1, 1, 1] + | ... +DROP TABLE t2; + | --- + | - row_count: 1 + | ... diff --git a/test/sql/add-column.test.sql b/test/sql/add-column.test.sql new file mode 100644 index 000000000..430c7c46a --- /dev/null +++ b/test/sql/add-column.test.sql @@ -0,0 +1,183 @@ +-- +-- gh-3075: Check statement. +-- +CREATE TABLE t1 (a INT PRIMARY KEY); + +-- +-- COLUMN keyword is optional. Check it here, but omit it below. +-- +ALTER TABLE t1 ADD COLUMN b INT; + +-- +-- A column with the same name already exists. +-- +ALTER TABLE t1 ADD b SCALAR; + +-- +-- Can't add column to a view. +-- +CREATE VIEW v AS SELECT * FROM t1; +ALTER TABLE v ADD c INT; + +\set language lua +view =3D box.space._space.index[2]:select('V')[1]:totable() +view_format =3D view[7] +f =3D {type =3D 'string', nullable_action =3D 'none', name =3D 'C', = is_nullable =3D true} +table.insert(view_format, f) +view[5] =3D 3 +view[7] =3D view_format +box.space._space:replace(view) +\set language sql + +DROP VIEW v; + +-- +-- Check PRIMARY KEY constraint works with an added column. +-- +CREATE TABLE pk_check (a INT CONSTRAINT pk PRIMARY KEY); +ALTER TABLE pk_check DROP CONSTRAINT pk; +ALTER TABLE pk_check ADD b INT PRIMARY KEY; +INSERT INTO pk_check VALUES (1, 1); +INSERT INTO pk_check VALUES (1, 1); +DROP TABLE pk_check; + +-- +-- Check UNIQUE constraint works with an added column. +-- +CREATE TABLE unique_check (a INT PRIMARY KEY); +ALTER TABLE unique_check ADD b INT UNIQUE; +INSERT INTO unique_check VALUES (1, 1); +INSERT INTO unique_check VALUES (2, 1); +DROP TABLE unique_check; + +-- +-- Check CHECK constraint works with an added column. +-- +CREATE TABLE ck_check (a INT PRIMARY KEY); +ALTER TABLE ck_check ADD b INT CHECK (b > 0); +INSERT INTO ck_check VALUES (1, 0); +INSERT INTO ck_check VALUES (1, 1); +DROP TABLE ck_check; + +-- +-- Check FOREIGN KEY constraint works with an added column. +-- +CREATE TABLE fk_check (a INT PRIMARY KEY); +ALTER TABLE fk_check ADD b INT REFERENCES t1(a); +INSERT INTO fk_check VALUES (0, 1); +INSERT INTO fk_check VALUES (2, 0); +INSERT INTO fk_check VALUES (2, 1); +INSERT INTO t1 VALUES (1, 1); +INSERT INTO fk_check VALUES (2, 1); +DROP TABLE fk_check; +DROP TABLE t1; +-- +-- Check FOREIGN KEY (self-referenced) constraint works with an +-- added column. +-- +CREATE TABLE self (id INT PRIMARY KEY AUTOINCREMENT, a INT UNIQUE) +ALTER TABLE self ADD b INT REFERENCES self(a) +INSERT INTO self(a,b) VALUES(1, 1); +UPDATE self SET a =3D 2, b =3D 2; +UPDATE self SET b =3D 3; +UPDATE self SET a =3D 3; +DROP TABLE self; + +-- +-- Check AUTOINCREMENT works with an added column. +-- +CREATE TABLE autoinc_check (a INT CONSTRAINT pk PRIMARY KEY); +ALTER TABLE autoinc_check DROP CONSTRAINT pk; +ALTER TABLE autoinc_check ADD b INT PRIMARY KEY AUTOINCREMENT; +INSERT INTO autoinc_check(a) VALUES(1); +INSERT INTO autoinc_check(a) VALUES(1); +TRUNCATE TABLE autoinc_check; + +-- +-- Can't add second column with AUTOINCREMENT. +-- +ALTER TABLE autoinc_check ADD c INT AUTOINCREMENT; +DROP TABLE autoinc_check; + +-- +-- Check COLLATE clause works with an added column. +-- +CREATE TABLE collate_check (a INT PRIMARY KEY); +ALTER TABLE collate_check ADD b TEXT COLLATE "unicode_ci"; +INSERT INTO collate_check VALUES (1, 'a'); +INSERT INTO collate_check VALUES (2, 'A'); +SELECT * FROM collate_check WHERE b LIKE 'a'; +DROP TABLE collate_check; + +-- +-- Check DEFAULT clause works with an added column. +-- +CREATE TABLE default_check (a INT PRIMARY KEY); +ALTER TABLE default_check ADD b TEXT DEFAULT ('a'); +INSERT INTO default_check(a) VALUES (1); +SELECT * FROM default_check; +DROP TABLE default_check; + +-- +-- Check NULL constraint works with an added column. +-- +CREATE TABLE null_check (a INT PRIMARY KEY); +ALTER TABLE null_check ADD b TEXT NULL; +INSERT INTO null_check(a) VALUES (1); +SELECT * FROM null_check; +DROP TABLE null_check; + +-- +-- Check NOT NULL constraint works with an added column. +-- +CREATE TABLE notnull_check (a INT PRIMARY KEY); +ALTER TABLE notnull_check ADD b TEXT NOT NULL; +INSERT INTO notnull_check(a) VALUES (1); +INSERT INTO notnull_check VALUES (1, 'not null'); +DROP TABLE notnull_check; + +-- +-- Can't add a column with DEAFULT or NULL to a non-empty space. +-- This ability isn't implemented yet. +-- +CREATE TABLE non_empty (a INT PRIMARY KEY); +INSERT INTO non_empty VALUES (1); +ALTER TABLE non_empty ADD b INT NULL; +ALTER TABLE non_empty ADD b INT DEFAULT (1); +DROP TABLE non_empty; + +-- +-- Add to a no-SQL adjusted space without format. +-- +\set language lua +_ =3D box.schema.space.create('WITHOUT_FORMAT') +\set language sql +ALTER TABLE without_format ADD a INT PRIMARY KEY; +INSERT INTO without_format VALUES (1); +DROP TABLE without_format; + +-- +-- Add to a no-SQL adjusted space with format. +-- +\set language lua +with_format =3D box.schema.space.create('WITH_FORMAT') +with_format:format{{name =3D 'A', type =3D 'unsigned'}} +\set language sql +ALTER TABLE with_format ADD b INT PRIMARY KEY; +INSERT INTO with_format VALUES (1, 1); +DROP TABLE with_format; + +-- +-- Add multiple columns (with a constraint) inside a transaction. +-- +CREATE TABLE t2 (a INT PRIMARY KEY) +\set language lua +box.begin() = \ +box.execute('ALTER TABLE t2 ADD b INT') = \ +box.execute('ALTER TABLE t2 ADD c INT UNIQUE') = \ +box.commit() +\set language sql +INSERT INTO t2 VALUES (1, 1, 1); +INSERT INTO t2 VALUES (2, 1, 1); +SELECT * FROM t2; +DROP TABLE t2; diff --git a/test/sql/checks.result b/test/sql/checks.result index 7b18e5d6b..3ddd52d42 100644 --- a/test/sql/checks.result +++ b/test/sql/checks.result @@ -856,6 +856,26 @@ box.execute("DROP TABLE t6") --- - row_count: 1 ... +-- +-- gh-3075: Check the auto naming of CHECK constraints in +-- . +-- +box.execute("CREATE TABLE check_naming (a INT PRIMARY KEY CHECK (a > = 0))") +--- +- row_count: 1 +... +box.execute("ALTER TABLE check_naming ADD b INT CHECK (b > 0)") +--- +- row_count: 1 +... +box.execute("ALTER TABLE check_naming DROP CONSTRAINT = \"ck_unnamed_CHECK_NAMING_2\"") +--- +- row_count: 1 +... +box.execute("DROP TABLE check_naming") +--- +- row_count: 1 +... test_run:cmd("clear filter") --- - true diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua index 301f8ea69..a79131466 100644 --- a/test/sql/checks.test.lua +++ b/test/sql/checks.test.lua @@ -280,4 +280,13 @@ box.func.MYFUNC:drop() box.execute("INSERT INTO t6 VALUES(11);"); box.execute("DROP TABLE t6") =20 +-- +-- gh-3075: Check the auto naming of CHECK constraints in +-- . +-- +box.execute("CREATE TABLE check_naming (a INT PRIMARY KEY CHECK (a > = 0))") +box.execute("ALTER TABLE check_naming ADD b INT CHECK (b > 0)") +box.execute("ALTER TABLE check_naming DROP CONSTRAINT = \"ck_unnamed_CHECK_NAMING_2\"") +box.execute("DROP TABLE check_naming") + test_run:cmd("clear filter") diff --git a/test/sql/foreign-keys.result b/test/sql/foreign-keys.result index 33689a06e..9ba995579 100644 --- a/test/sql/foreign-keys.result +++ b/test/sql/foreign-keys.result @@ -499,5 +499,33 @@ box.space.S:drop() box.space.T:drop() --- ... +-- +-- gh-3075: Check the auto naming of FOREIGN KEY constraints in +-- . +-- +box.execute("CREATE TABLE t1 (a INT PRIMARY KEY)") +--- +- row_count: 1 +... +box.execute("CREATE TABLE check_naming (a INT PRIMARY KEY REFERENCES = t1(a))") +--- +- row_count: 1 +... +box.execute("ALTER TABLE check_naming ADD b INT REFERENCES t1(a)") +--- +- row_count: 1 +... +box.execute("ALTER TABLE check_naming DROP CONSTRAINT = \"fk_unnamed_CHECK_NAMING_2\"") +--- +- row_count: 1 +... +box.execute("DROP TABLE check_naming") +--- +- row_count: 1 +... +box.execute("DROP TABLE t1") +--- +- row_count: 1 +... --- Clean-up SQL DD hash. -test_run:cmd('restart server default with cleanup=3D1') diff --git a/test/sql/foreign-keys.test.lua = b/test/sql/foreign-keys.test.lua index d2dd88d28..29918c5d4 100644 --- a/test/sql/foreign-keys.test.lua +++ b/test/sql/foreign-keys.test.lua @@ -209,5 +209,16 @@ box.space.T:select() box.space.S:drop() box.space.T:drop() =20 +-- +-- gh-3075: Check the auto naming of FOREIGN KEY constraints in +-- . +-- +box.execute("CREATE TABLE t1 (a INT PRIMARY KEY)") +box.execute("CREATE TABLE check_naming (a INT PRIMARY KEY REFERENCES = t1(a))") +box.execute("ALTER TABLE check_naming ADD b INT REFERENCES t1(a)") +box.execute("ALTER TABLE check_naming DROP CONSTRAINT = \"fk_unnamed_CHECK_NAMING_2\"") +box.execute("DROP TABLE check_naming") +box.execute("DROP TABLE t1") + --- Clean-up SQL DD hash. -test_run:cmd('restart server default with cleanup=3D1')