From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) (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 64EA1446439 for ; Tue, 10 Nov 2020 15:18:02 +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: <20201106155719.GA13328@tarantool.org> Date: Tue, 10 Nov 2020 15:18:00 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <8AFFA6D9-CCF7-4134-9018-0C6389542ADE@tarantool.org> References: <20201009134529.13212-1-roman.habibov@tarantool.org> <20201009134529.13212-6-roman.habibov@tarantool.org> <20201106155719.GA13328@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 5/5] 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 Hi! Thanks for the review. > On Nov 6, 2020, at 18:57, Nikita Pettik = wrote: >=20 > On 09 Oct 16:45, Roman Khabibov wrote: >> 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 > Extra empty line. Removed. >> /* >> * !IMPORTANT! Please follow instructions at start of the file >> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >> index fd8e7ed1e..597608bea 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 >> +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; >> + } >=20 >=20 > You don't need all indexes from space..What you only need from space - = =20 > count of indexes (to generate name if it is required). The same for > field defs. Mb it is worth avoiding copying space at all - just > store in create_column_def list of ck/fk constraints, single > field_def, index/field count - these objects seem to be enough. > Please consider this suggestion. Yes, I understand that. But unfortunately, the current code, namely the functions for creating indexes, fk, and ck constraints, are written in such a way that they cannot be called without struct space (ephemeral or not). Vlad and I discussed this and came to the conclusion that it is better to go in the direction of reusing the code, rather than rewriting a significant part of build.c. = https://lists.tarantool.org/pipermail/tarantool-patches/2020-July/018460.h= tml = https://lists.tarantool.org/pipermail/tarantool-patches/2020-August/019003= .html >> + >> + 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; >=20 > assert(is_alter); If you meant if (def->exact_field_count =3D=3D 0) { assert(is_alter); def->exact_field_count =3D def->field_count; } This branch is not only for alter. >> + struct Vdbe *v =3D sqlGetVdbe(parse); >> + assert(v !=3D NULL); >> + >> + struct space *system_space =3D space_by_id(BOX_SPACE_ID); >=20 > -> you can call it simply _space or s_space Done. >> + assert(system_space !=3D NULL); >> + int cursor =3D parse->nTab++; >> + vdbe_emit_open_cursor(parse, cursor, 0, system_space); >> + sqlVdbeChangeP5(v, OPFLAG_SYSTEMSP); >> + >> 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; >=20 > Let's use standard facilities: assert(def->field_count > 0); Done. 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)) - return; - struct space_def *def =3D space->def; + assert(parser->create_column_def.space !=3D NULL); + struct space_def *def =3D parser->create_column_def.space->def; + assert(def->field_count > 0); 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) { >> 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; >=20 > Consider tiny refactoring: >=20 > is_alter =3D space =3D=3D NULL; > if (is_alter) > space =3D ... > assert(space !=3D NULL); In my case it will be: struct space *space =3D parser->create_column_def.space; bool is_alter_add_constr =3D space =3D=3D NULL && parser->create_table_def.new_space =3D=3D = NULL; if (!is_alter_add_constr) space =3D parser->parser->create_table_def.new_space; This results in more rows than there are now. >> + if (space =3D=3D NULL) >> + space =3D parser->create_table_def.new_space; >> + bool is_alter_add_constr =3D space =3D=3D NULL; >>=20 >> - assert(! is_alter); >> + assert(!is_alter_add_constr); >=20 > What's the point of this renaming? I need to distinguish between , and branches. >=20 >> uint32_t ck_idx =3D ++parser->create_checks_def.count; >> + /* >> @@ -1149,15 +1302,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 >> vdbe_emit_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 >=20 > AFAIK only one index (corresponding to unique constraint). I can write any number of named unique constraints. They should be created. ALTER TABLE t ADD COLUMN a INT CONSTRAINT c_1 UNIQUE, CONSTRAINT c_2 = UNIQUE =E2=80=A6 > =20 >=20 >> + * 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; >> + } >> @@ -1176,6 +1342,21 @@ vdbe_emit_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) >=20 > Oh please leave !=3D0 cond on previous line. This way looks = disgusting. Done. >> + return; >> + } >> sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, = reg_seq_rec); >> /* Do an insertion into _space_sequence. */ >> int reg_space_seq_record =3D >> + * CONSTRAINT> statement handling. >> */ >> - bool is_alter =3D space =3D=3D NULL; >> + bool is_alter_add_constr =3D space =3D=3D NULL; >=20 > Again, what's the point of this renaming? commit d00ac8986bd37d5234e7d3889cb6f77216c1d33c 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..cfea5bcf2 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -273,6 +273,7 @@ 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 fd8e7ed1e..35883aa0f 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,85 @@ 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 +vdbe_emit_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 *s_space =3D space_by_id(BOX_SPACE_ID); + assert(s_space !=3D NULL); + int cursor =3D parse->nTab++; + vdbe_emit_open_cursor(parse, cursor, 0, s_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 *) s_space, P4_SPACEPTR); + sqlVdbeCountChanges(v); + sqlVdbeChangeP5(v, OPFLAG_NCHANGE); + sqlReleaseTempRange(parse, tuple_reg, box_space_field_MAX + 1); + vdbe_emit_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)) - return; - struct space_def *def =3D space->def; + assert(parser->create_column_def.space !=3D NULL); + struct space_def *def =3D parser->create_column_def.space->def; + assert(def->field_count > 0); 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 +493,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 +577,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 +706,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 +723,22 @@ sql_create_check_contraint(struct Parse *parser) return; } } else { - assert(! is_alter); + assert(!is_alter_add_constr); uint32_t ck_idx =3D ++parser->create_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_%u", = space->def->name, ck_idx); } size_t name_len =3D strlen(name); @@ -634,7 +782,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) { @@ -664,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); @@ -696,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. @@ -705,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; @@ -795,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 { @@ -1033,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 + * vdbe_emit_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 { @@ -1108,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); } @@ -1149,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 vdbe_emit_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, @@ -1176,6 +1341,20 @@ vdbe_emit_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 @@ -1874,24 +2053,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) { @@ -1909,6 +2092,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->create_fkeys_def.fkeys, fk_parse, link); } @@ -1922,27 +2111,41 @@ 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->create_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 rlist *fkeys =3D = &parse_context->create_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; } - if (!is_alter) { + if (!is_alter_add_constr) { if (create_def->name.n =3D=3D 0) { uint32_t idx =3D = ++parse_context->create_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_%u", space->def->name, = idx); } else { @@ -2024,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); /* @@ -2053,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 vdbe_emit_create_constraints() call and + * simply maintain list of all FK constraints inside + * parser. */ - if (!is_alter) { + if (!is_alter_add_constr) { struct rlist *fkeys =3D = &parse_context->create_fkeys_def.fkeys; struct fk_constraint_parse *fk_parse =3D rlist_first_entry(fkeys, struct = fk_constraint_parse, @@ -2435,7 +2640,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); @@ -2448,10 +2656,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 @@ -2486,7 +2692,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) { @@ -2652,7 +2858,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; @@ -2740,7 +2946,8 @@ sql_create_index(struct Parse *parse) { sqlVdbeAddOp0(vdbe, OP_Expire); } =20 - if (tbl_name !=3D NULL || sql_space_add_index(parse, space, = index) !=3D 0) + if (!is_create_table_or_add_col || + sql_space_add_index(parse, space, index) !=3D 0) goto exit_create_index; index =3D NULL; =20 diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index da4e2a3ae..d7c65bb4b 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->create_checks_def); + create_fkeys_def_init(&pParse->create_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 21829b6f0..07230a8be 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 { /** List of ck_constraint_parse objects. */ struct rlist checks; @@ -478,6 +488,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 2e426600d..a54aa1d51 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 create_fkeys_def; struct create_checks_def create_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')