From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <roman.habibov@tarantool.org>
Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149])
(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 B594743040B
for <tarantool-patches@dev.tarantool.org>;
Tue, 11 Aug 2020 03:34:06 +0300 (MSK)
Content-Type: text/plain;
charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\))
From: Roman Khabibov <roman.habibov@tarantool.org>
In-Reply-To: <cf5ab6b6-0d5a-e542-03f8-9614a377880f@tarantool.org>
Date: Tue, 11 Aug 2020 03:34:00 +0300
Content-Transfer-Encoding: quoted-printable
Message-Id: <3FF95208-4C8E-4094-A005-7A50415A8651@tarantool.org>
References: <20200403152752.8923-1-roman.habibov@tarantool.org>
<20200403152752.8923-3-roman.habibov@tarantool.org>
<d99bcaca-b98e-f8d0-8a54-d86b8812697e@tarantool.org>
<5CF72787-A1F0-4C48-BA8F-08F02B6960F6@tarantool.org>
<cf5ab6b6-0d5a-e542-03f8-9614a377880f@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v2 2/2] sql: support column addition
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Hi! Thanks for the review.
> On Jul 12, 2020, at 19:45, Vladislav Shpilevoy =
<v.shpilevoy@tarantool.org> wrote:
>=20
> Hi! Thanks for the patch!
>=20
> See 33 comments below.
>=20
>> commit 82c448dc66f6233faeb40dda353652c2fd5a3d29
>> Author: Roman Khabibov <roman.habibov@tarantool.org>
>> Date: Thu Jan 2 19:06:14 2020 +0300
>>=20
>> sql: support column addition
>>=20
>> Enable to add column to existing space with
>> <ALTER TABLE ADD [COLUMN]> statement. Column definition can be
>> supplemented with the four types of constraints, <DEFAULT>,
>> <COLLATE> 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
>> <ALTER TABLE table_name ADD [COLUMN] column_name column_type ...>
>> statement. The column definition is the same as in <CREATE TABLE>
>> statement.
>>=20
>> For example:
>>=20
>> tarantool> box.execute([[CREATE TABLE test (
>> a INTEGER PRIMARY KEY
>> );]])
>> ---
>> - row_count: 1
>> ...
>>=20
>> tarantool> box.execute([[ALTER TABLE test ADD COLUMN
>> b TEXT
>> NOT NULL
>> DEFAULT ('a')
>> COLLATE "unicode_ci"
>> ;]])
>> ---
>> - row_count: 0
>> ...
>> ---
>=20
> 1. The commit message is different from what I see on the branch. On =
the
> branch there is no 'Closes #2349'. What is the most actual version?
>=20
> 2. The example from the doc request does not work:
>=20
> tarantool> box.execute([[CREATE TABLE test (a INTEGER PRIMARY =
KEY);]])
> ---
> - row_count: 1
> ...
>=20
> tarantool> box.execute([[ALTER TABLE test ADD COLUMN b TEXT NOT =
NULL DEFAULT ('a') COLLATE "unicode_ci";]])
> ---
> - null
> - 'At line 1 at or near position 22: keyword ''COLUMN'' is =
reserved. Please use double
> quotes if ''COLUMN'' is an identifier.'
> ...
>=20
>> diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
>> index 486b6b30d..dea047241 100644
>> --- a/extra/mkkeywordhash.c
>> +++ b/extra/mkkeywordhash.c
>> @@ -76,7 +76,7 @@ static Keyword aKeywordTable[] =3D {
>> { "CHECK", "TK_CHECK", true },
>> { "COLLATE", "TK_COLLATE", true },
>> { "COLUMN_NAME", "TK_COLUMN_NAME", true },
>> - { "COLUMN", "TK_STANDARD", true },
>> + { "COLUMN", "TK_COLUMN", true },
>=20
> 3. This is how the same hunk looks on the branch:
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
> index 006285622..51c8cbb18 100644
> --- a/extra/mkkeywordhash.c
> +++ b/extra/mkkeywordhash.c
> @@ -75,11 +75,7 @@ static Keyword aKeywordTable[] =3D {
> { "CAST", "TK_CAST", false },
> { "CHECK", "TK_CHECK", true },
> { "COLLATE", "TK_COLLATE", true },
> - /* gh-3075: Reserved until ALTER ADD COLUMN is implemeneted.
> - * Move it back to ALTER when done.
> - */
> - /* { "COLUMN", "TK_COLUMNKW", true }, */
> - { "COLUMN", "TK_STANDARD", true },
> + { "COLUMN", "TK_COLUMN", true },
> { "COMMIT", "TK_COMMIT", true },
> { "CONFLICT", "TK_CONFLICT", false },
> { "CONSTRAINT", "TK_CONSTRAINT", true },
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> It is very different. Why?
>=20
>> diff --git a/src/box/errcode.h b/src/box/errcode.h
>> index d1e4d02a9..3e94bee7a 100644
>> --- a/src/box/errcode.h
>> +++ b/src/box/errcode.h
>> @@ -266,6 +266,8 @@ struct errcode_record {
>> /*211 */_(ER_WRONG_QUERY_ID, "Prepared statement with =
id %u does not exist") \
>> /*212 */_(ER_SEQUENCE_NOT_STARTED, "Sequence '%s' =
is not started") \
>> /*213 */_(ER_NO_SUCH_SESSION_SETTING, "Session setting %s =
doesn't exist") \
>> + /*214 */_(ER_SQL_CANT_ADD_COLUMN_TO_VIEW, "Can't add =
column '%s'. '%s' is a view") \
>> + /*215 */_(ER_SQL_CANT_ADD_AUTOINC, "Can't add =
AUTOINCREMENT: the space '%s' already has one auto-incremented field") \
>=20
> 4. The same here:
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -264,6 +264,7 @@ struct errcode_record {
> /*209 */_(ER_SESSION_SETTING_INVALID_VALUE, "Session =
setting %s expected a value of type %s") \
> /*210 */_(ER_SQL_PREPARE, "Failed to prepare SQL =
statement: %s") \
> /*211 */_(ER_WRONG_QUERY_ID, "Prepared statement =
with id %u does not exist") \
> + /*212 */_(ER_SQL_CANT_ADD_COLUMN_TO_VIEW, "Can't add =
column '%s' to the view '%s'") \
>=20
> /*
> * !IMPORTANT! Please follow instructions at start of the file
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> =46rom this point I will review the email, not the branch.
> Generally the patch still looks very raw. I hope this is mostly
> because I couldn't review it properly locally.
There was two branches on GitHub: romanhabibov/gh-3075-add-column and =
romanhabibov/gh-3075-add-column-v2.
I dropped the first so we don't get confused anymore.
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index ac42fe842..45fb90d38 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -285,48 +285,112 @@ sql_field_retrieve(Parse *parser, struct =
space_def *space_def, uint32_t id)
>> return field;
>> }
>>=20
>> -/*
>> - * Add a new column to the table currently being constructed.
>> +/**
>> + * Make shallow copy of @a space on region.
>> *
>> - * The parser calls this routine once for each column declaration
>> - * in a CREATE TABLE statement. sqlStartTable() gets called
>> - * first to get things going. Then this routine is called for each
>> - * column.
>> + * Function is used to add a new column to the existing space with
>> + * <ALTER TABLE ADD COLUMN>. Copy info about indexes and
>> + * definition to create constraints appeared in the statement.
>=20
> 5. I don't think I understood anything from the comment. Why is it =
needed
> to create a copy (I remember why, a bit, but I mostly forgot it)?
To reuse the existing code. This is the best way, I think. Otherwise, =
we
will have to essentially rewrite the functions for creating constraints.
-/*
- * 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
+ * <ALTER TABLE ADD COLUMN> 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)
>> */
>> +static struct space *
>> +sql_shallow_space_copy(struct Parse *parse, struct space *space)
>> +{
>> + assert(space->def !=3D NULL);
>> + struct space *ret =3D sql_ephemeral_space_new(parse, =
space->def->name);
>> + if (ret =3D=3D NULL)
>> + return NULL;
>> + ret->index_count =3D space->index_count;
>> + ret->index_id_max =3D space->index_id_max;
>> + uint32_t indexes_sz =3D sizeof(struct index *) * =
(ret->index_count);
>> + ret->index =3D (struct index **) malloc(indexes_sz);
>=20
> 6. When is this array freed?
In sql_create_column_end(struct Parse *parse)
+ /*
+ * Clean up array allocated in sql_shallow_space_copy().
+ */
+ free(space->index);
>> + if (ret->index =3D=3D NULL) {
>> + diag_set(OutOfMemory, indexes_sz, "realloc", =
"ret->index");
>> + return NULL;
>> + }
>> + for (uint32_t i =3D 0; i < ret->index_count; i++)
>> + ret->index[i] =3D space->index[i];
>=20
> 7. What is the problem to make memcpy() instead of the cycle?
No problem. See below.
>> + memcpy(ret->def, space->def, sizeof(struct space_def));
>> + ret->def->opts.is_temporary =3D true;
>> + ret->def->opts.is_ephemeral =3D true;
>> + uint32_t fields_size =3D sizeof(struct field_def) * =
ret->def->field_count;
>> + ret->def->fields =3D region_alloc(&parse->region, fields_size);
>=20
> 8. Use region_alloc_array. Otherwise it will crash in ASAN build. It
> should be visible in CI.
Ok.
>> + if (ret->def->fields =3D=3D NULL) {
>> + diag_set(OutOfMemory, fields_size, "region_alloc",
>> + "ret->def->fields");
>=20
> 9. index array leaks here.
Fixed.
+static struct space *
+sql_shallow_space_copy(struct Parse *parse, struct space *space)
+{
+ assert(space->def !=3D NULL);
+ struct space *ret =3D sql_ephemeral_space_new(parse, =
space->def->name);
+ if (ret =3D=3D NULL)
+ return NULL;
+ ret->index_count =3D space->index_count;
+ ret->index_id_max =3D space->index_id_max;
+ uint32_t indexes_sz =3D sizeof(struct index *) * =
(ret->index_count);
+ ret->index =3D (struct index **) malloc(indexes_sz);
+ if (ret->index =3D=3D NULL) {
+ diag_set(OutOfMemory, indexes_sz, "realloc", =
"ret->index");
+ return NULL;
+ }
+ memcpy(ret->index, space->index, indexes_sz);
+ 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");
+ free(ret->index);
+ return NULL;
+ }
+ memcpy(ret->def->fields, space->def->fields, =
fields_size);
+ }
+
+ return ret;
+}
>> + return NULL;
>> + }
>> + memcpy(ret->def->fields, space->def->fields, fields_size);
>> +
>> + return ret;
>> +}
>> +
>> void
>> -sqlAddColumn(Parse * pParse, Token * pName, struct type_def =
*type_def)
>> +sql_create_column_start(struct Parse *parse)
>> {
>> - assert(type_def !=3D NULL);
>> - char *z;
>> - sql *db =3D pParse->db;
>> - if (pParse->create_table_def.new_space =3D=3D NULL)
>> - return;
>> - struct space_def *def =3D =
pParse->create_table_def.new_space->def;
>> + struct create_column_def *create_column_def =3D =
&parse->create_column_def;
>> + struct alter_entity_def *alter_entity_def =3D
>> + &create_column_def->base.base;
>> + assert(alter_entity_def->entity_type =3D=3D ENTITY_TYPE_COLUMN);
>> + assert(alter_entity_def->alter_action =3D=3D =
ALTER_ACTION_CREATE);
>> + struct space *space =3D parse->create_table_def.new_space;
>> + bool is_alter =3D space =3D=3D NULL;
>> + struct sql *db =3D parse->db;
>> + if (is_alter) {
>> + const char *space_name =3D
>> + alter_entity_def->entity_name->a[0].zName;
>> + space =3D space_by_name(space_name);
>> + if (space =3D=3D NULL) {
>> + diag_set(ClientError, ER_NO_SUCH_SPACE, =
space_name);
>> + goto tnt_error;
>> + }
>> + space =3D sql_shallow_space_copy(parse, space);
>> + if (space =3D=3D NULL)
>> + goto tnt_error;
>> + }
>> + create_column_def->space =3D space;
>> + struct space_def *def =3D space->def;
>> + assert(def->opts.is_ephemeral);
>>=20
>> #if SQL_MAX_COLUMN
>> if ((int)def->field_count + 1 > db->aLimit[SQL_LIMIT_COLUMN]) {
>> diag_set(ClientError, ER_SQL_COLUMN_COUNT_MAX, =
def->name,
>> def->field_count + 1, =
db->aLimit[SQL_LIMIT_COLUMN]);
>> - pParse->is_aborted =3D true;
>> - return;
>> + goto tnt_error;
>=20
> 10. Was there a leak before your patch? Because of not
> called sqlSrcListDelete(db, alter_entity_def->entity_name);
No, because alter_entity_def is a part of create_column_def.
create_column_def didn=E2=80=99t exist before my patch.
>> }
>> #endif
>> +
>> + struct region *region =3D &parse->region;
>> + struct Token *name =3D &create_column_def->base.name;
>> + char *column_name =3D
>> + sql_normalized_name_region_new(region, name->z, =
name->n);
>> + if (column_name =3D=3D NULL)
>> + goto tnt_error;
>> +
>> + if (parse->create_table_def.new_space =3D=3D NULL && =
def->opts.is_view) {
>=20
> 11. You have is_alter flag for that.
Yes. Fixed.
>> + diag_set(ClientError, ER_SQL_CANT_ADD_COLUMN_TO_VIEW,
>> + column_name, def->name);
>> + goto tnt_error;
>> + }
>> +
>> /*
>> - * As sql_field_retrieve will allocate memory on region
>> - * ensure that def is also temporal and would be dropped.
>> + * Format can be set in Lua, then exact_field_count can be
>> + * zero, but field_count is not.
>> */
>> - assert(def->opts.is_ephemeral);
>> - if (sql_field_retrieve(pParse, def, def->field_count) =3D=3D =
NULL)
>> - return;
>> - struct region *region =3D &pParse->region;
>> - z =3D sql_normalized_name_region_new(region, pName->z, =
pName->n);
>> - if (z =3D=3D NULL) {
>> - pParse->is_aborted =3D true;
>> + if (def->exact_field_count =3D=3D 0)
>> + def->exact_field_count =3D def->field_count;
>> + if (sql_field_retrieve(parse, def, def->field_count) =3D=3D =
NULL)
>> return;
>> +
>> + for (uint32_t i =3D 0; i < def->field_count; i++) {
>> + if (strcmp(column_name, def->fields[i].name) =3D=3D 0) {
>> + diag_set(ClientError, =
ER_SPACE_FIELD_IS_DUPLICATE,
>> + column_name);
>> + goto tnt_error;
>> + }
>=20
> 12. I remember this code was deliberately removed, because the same =
check is
> already done in box. Why did you return it?
Sorry, removed.
>> }
>> struct field_def *column_def =3D &def->fields[def->field_count];
>> - memcpy(column_def, &field_def_default, =
sizeof(field_def_default));
>> - column_def->name =3D z;
>> + memcpy(column_def, &field_def_default, sizeof(struct =
field_def));
>=20
> 13. Unnecessary change of memcpy().
Yes. Returned old version.
+sql_create_column_start(struct Parse *parse)
{
- assert(type_def !=3D NULL);
- char *z;
- sql *db =3D pParse->db;
- if (pParse->create_table_def.new_space =3D=3D NULL)
- return;
- struct space_def *def =3D =
pParse->create_table_def.new_space->def;
+ struct create_column_def *create_column_def =3D =
&parse->create_column_def;
+ struct alter_entity_def *alter_entity_def =3D
+ &create_column_def->base.base;
+ assert(alter_entity_def->entity_type =3D=3D ENTITY_TYPE_COLUMN);
+ assert(alter_entity_def->alter_action =3D=3D =
ALTER_ACTION_CREATE);
+ struct space *space =3D parse->create_table_def.new_space;
+ bool is_alter =3D space =3D=3D NULL;
+ struct sql *db =3D parse->db;
+ if (is_alter) {
+ const char *space_name =3D
+ alter_entity_def->entity_name->a[0].zName;
+ space =3D space_by_name(space_name);
+ if (space =3D=3D NULL) {
+ diag_set(ClientError, ER_NO_SUCH_SPACE, =
space_name);
+ goto tnt_error;
+ }
+ space =3D sql_shallow_space_copy(parse, space);
+ if (space =3D=3D NULL)
+ goto tnt_error;
+ }
+ create_column_def->space =3D space;
+ struct space_def *def =3D space->def;
+ assert(def->opts.is_ephemeral);
=20
#if SQL_MAX_COLUMN
if ((int)def->field_count + 1 > db->aLimit[SQL_LIMIT_COLUMN]) {
diag_set(ClientError, ER_SQL_COLUMN_COUNT_MAX, =
def->name,
def->field_count + 1, =
db->aLimit[SQL_LIMIT_COLUMN]);
- pParse->is_aborted =3D true;
- return;
+ goto tnt_error;
}
#endif
+
+ struct region *region =3D &parse->region;
+ struct Token *name =3D &create_column_def->base.name;
+ char *column_name =3D
+ sql_normalized_name_region_new(region, name->z, =
name->n);
+ if (column_name =3D=3D NULL)
+ goto tnt_error;
+
+ if (is_alter && def->opts.is_view) {
+ diag_set(ClientError, ER_SQL_CANT_ADD_COLUMN_TO_VIEW,
+ column_name, def->name);
+ goto tnt_error;
+ }
+
/*
- * As sql_field_retrieve will allocate memory on region
- * ensure that def is also temporal and would be dropped.
+ * Format can be set in Lua, then exact_field_count can be
+ * zero, but field_count is not.
*/
- assert(def->opts.is_ephemeral);
- if (sql_field_retrieve(pParse, def, def->field_count) =3D=3D =
NULL)
+ 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 +399,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);
+}
+
>> + column_def->name =3D column_name;
>> /*
>> * Marker ON_CONFLICT_ACTION_DEFAULT is used to detect
>> * attempts to define NULL multiple time or to detect
>> @@ -334,19 +398,100 @@ sqlAddColumn(Parse * pParse, Token * pName, =
struct type_def *type_def)
>> */
>> column_def->nullable_action =3D ON_CONFLICT_ACTION_DEFAULT;
>> column_def->is_nullable =3D true;
>> - column_def->type =3D type_def->type;
>> + column_def->type =3D create_column_def->type_def->type;
>> def->field_count++;
>> +
>> + sqlSrcListDelete(db, alter_entity_def->entity_name);
>> + return;
>> +tnt_error:
>> + parse->is_aborted =3D true;
>> + sqlSrcListDelete(db, alter_entity_def->entity_name);
>> +}
>> +
>> +static void
>> +sql_vdbe_create_constraints(struct Parse *parse, int reg_space_id);
>> +
>> +void
>> +sql_create_column_end(struct Parse *parse)
>> +{
>> + struct create_column_def *create_column_def =3D =
&parse->create_column_def;
>> + struct space *space =3D parse->create_table_def.new_space;
>> + bool is_alter =3D space =3D=3D NULL;
>> + space =3D create_column_def->space;
>> + struct space_def *def =3D space->def;
>> + if (is_alter) {
>> + struct field_def *field =3D =
&def->fields[def->field_count - 1];
>> + if (field->nullable_action =3D=3D =
ON_CONFLICT_ACTION_DEFAULT) {
>> + if (create_column_def->is_pk) {
>> + field->nullable_action =3D
>> + ON_CONFLICT_ACTION_ABORT;
>> + field->is_nullable =3D false;
>> + } else {
>> + field->nullable_action =3D
>> + ON_CONFLICT_ACTION_NONE;
>> + field->is_nullable =3D true;
>> + }
>> + }
>> + /*
>> + * Encode the format array and emit code to update =
_space.
>> + */
>> + uint32_t table_stmt_sz =3D 0;
>> + struct region *region =3D &parse->region;
>> + char *table_stmt =3D sql_encode_table(region, def,
>> + &table_stmt_sz);
>> + char *raw =3D sqlDbMallocRaw(parse->db, table_stmt_sz);
>> + if (table_stmt =3D=3D NULL || raw =3D=3D NULL) {
>> + parse->is_aborted =3D true;
>> + return;
>> + }
>> + memcpy(raw, table_stmt, table_stmt_sz);
>> +
>> + struct Vdbe *v =3D sqlGetVdbe(parse);
>> + assert(v !=3D NULL);
>> +
>> + struct space *system_space =3D =
space_by_id(BOX_SPACE_ID);
>> + assert(system_space !=3D NULL);
>> + int cursor =3D parse->nTab++;
>> + vdbe_emit_open_cursor(parse, cursor, 0, system_space);
>> + sqlVdbeChangeP5(v, OPFLAG_SYSTEMSP);
>> +
>> + int key_reg =3D ++parse->nMem;
>> + sqlVdbeAddOp2(v, OP_Integer, def->id, key_reg);
>> + int addr =3D sqlVdbeAddOp4Int(v, OP_Found, cursor, 0, =
key_reg, 1);
>> + sqlVdbeAddOp2(v, OP_Halt, -1, ON_CONFLICT_ACTION_ABORT);
>> + sqlVdbeJumpHere(v, addr);
>> +
>> + int tuple_reg =3D sqlGetTempRange(parse, =
box_space_field_MAX + 1);
>> + for (int i =3D 0; i < box_space_field_MAX - 1; ++i)
>> + sqlVdbeAddOp3(v, OP_Column, cursor, i, tuple_reg =
+ i);
>> + sqlVdbeAddOp1(v, OP_Close, cursor);
>> +
>> + sqlVdbeAddOp2(v, OP_Integer, def->field_count, tuple_reg =
+ 4);
>> + sqlVdbeAddOp4(v, OP_Blob, table_stmt_sz, tuple_reg + 6,
>> + SQL_SUBTYPE_MSGPACK, raw, P4_DYNAMIC);
>> + sqlVdbeAddOp3(v, OP_MakeRecord, tuple_reg, =
box_space_field_MAX,
>> + tuple_reg + box_space_field_MAX);
>> + sqlVdbeAddOp4(v, OP_IdxReplace, tuple_reg + =
box_space_field_MAX,
>> + 0, 0, (char *) system_space, P4_SPACEPTR);
>> + sql_vdbe_create_constraints(parse, key_reg);
>> + }
>> +
>> + memset(create_column_def, 0, sizeof(struct create_column_def));
>> + create_column_def->nullable_action =3D =
ON_CONFLICT_ACTION_DEFAULT;
>=20
> 14. Why do you touch column creation def after its usage?
Removed.
+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 + 4);
+ sqlVdbeAddOp4(v, OP_Blob, table_stmt_sz, tuple_reg + 6,
+ SQL_SUBTYPE_MSGPACK, raw, P4_DYNAMIC);
+ sqlVdbeAddOp3(v, OP_MakeRecord, tuple_reg, box_space_field_MAX,
+ tuple_reg + box_space_field_MAX);
+ sqlVdbeAddOp4(v, OP_IdxReplace, tuple_reg + box_space_field_MAX, =
0, 0,
+ (char *) system_space, P4_SPACEPTR);
+ sql_vdbe_create_constraints(parse, key_reg);
+
+ /*
+ * Clean up array allocated in sql_shallow_space_copy().
+ */
+ free(space->index);
}
>> }
>>=20
>> void
>> sql_column_add_nullable_action(struct Parse *parser,
>> enum on_conflict_action nullable_action)
>> {
>> - struct space *space =3D parser->create_table_def.new_space;
>> - if (space =3D=3D NULL || NEVER(space->def->field_count < 1))
>> + struct space_def *def =3D NULL;
>> + struct field_def *field =3D NULL;
>> + struct space *space =3D parser->create_column_def.space;
>> + assert (space !=3D NULL);
>=20
> 15. Please, don't put whitespace after macro/function name and its =
arguments.
Sorry. Fixed.
>> + def =3D space->def;
>> + if (NEVER(def->field_count < 1))
>> return;
>> - struct space_def *def =3D space->def;
>> - struct field_def *field =3D &def->fields[def->field_count - 1];
>> + field =3D &def->fields[def->field_count - 1];
>=20
> 16. Why did you do that change about struct field_def? It seems not to
> be needed.
Yes.
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,51 +497,42 @@ sql_column_add_nullable_action(struct Parse =
*parser,
}
>=20
>> if (field->nullable_action !=3D ON_CONFLICT_ACTION_DEFAULT &&
>> nullable_action !=3D field->nullable_action) {
>> /* Prevent defining nullable_action many times. */
>> @@ -364,51 +509,46 @@ sql_column_add_nullable_action(struct Parse =
*parser,
>> }
>>=20
>> /*
>> - * The expression is the default value for the most recently added =
column
>> - * of the table currently under construction.
>> + * The expression is the default value for the most recently added
>> + * column.
>> *
>> * Default value expressions must be constant. Raise an exception if =
this
>> * is not the case.
>> *
>> * This routine is called by the parser while in the middle of
>> - * parsing a CREATE TABLE statement.
>> + * parsing a <CREATE TABLE> or a <ALTER TABLE ADD COLUMN>
>> + * statement.
>> */
>> void
>> sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan)
>> {
>> sql *db =3D pParse->db;
>> - struct space *p =3D pParse->create_table_def.new_space;
>> - if (p !=3D NULL) {
>> - assert(p->def->opts.is_ephemeral);
>> - struct space_def *def =3D p->def;
>> - if (!sqlExprIsConstantOrFunction
>> - (pSpan->pExpr, db->init.busy)) {
>> - const char *column_name =3D
>> - def->fields[def->field_count - 1].name;
>> - diag_set(ClientError, ER_CREATE_SPACE, =
def->name,
>> - tt_sprintf("default value of column =
'%s' is "\
>> - "not constant", =
column_name));
>> + struct space_def *def =3D NULL;
>> + struct field_def *field =3D NULL;
>> + struct space *space =3D pParse->create_column_def.space;
>> + assert (space !=3D NULL);
>> + def =3D space->def;
>=20
> 17. Why can't you declare and initialize space_def in one line? The =
same
> for field.
Fixed.
void
sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan)
{
sql *db =3D pParse->db;
- struct space *p =3D pParse->create_table_def.new_space;
- if (p !=3D NULL) {
- assert(p->def->opts.is_ephemeral);
- struct space_def *def =3D p->def;
- if (!sqlExprIsConstantOrFunction
- (pSpan->pExpr, db->init.busy)) {
- const char *column_name =3D
- def->fields[def->field_count - 1].name;
- diag_set(ClientError, ER_CREATE_SPACE, =
def->name,
- tt_sprintf("default value of column =
'%s' is "\
- "not constant", =
column_name));
+ assert(pParse->create_column_def.space !=3D NULL);
+ struct space_def *def =3D pParse->create_column_def.space->def;
+ struct field_def *field =3D &def->fields[def->field_count - 1];
+ if (!sqlExprIsConstantOrFunction(pSpan->pExpr, db->init.busy)) {
+ diag_set(ClientError, ER_CREATE_SPACE, def->name,
+ tt_sprintf("default value of column '%s' is not =
"
+ "constant", field->name));
+ pParse->is_aborted =3D true;
+ } else {
+ struct region *region =3D &pParse->region;
+ uint32_t default_length =3D (int)(pSpan->zEnd - =
pSpan->zStart);
+ field->default_value =3D region_alloc(region, =
default_length + 1);
+ if (field->default_value =3D=3D NULL) {
+ diag_set(OutOfMemory, default_length + 1,
+ "region_alloc", =
"field->default_value");
pParse->is_aborted =3D true;
- } else {
- assert(def !=3D NULL);
- struct field_def *field =3D
- &def->fields[def->field_count - 1];
- struct region *region =3D &pParse->region;
- uint32_t default_length =3D (int)(pSpan->zEnd - =
pSpan->zStart);
- field->default_value =3D region_alloc(region,
- =
default_length + 1);
- if (field->default_value =3D=3D NULL) {
- diag_set(OutOfMemory, default_length + =
1,
- "region_alloc",
- "field->default_value");
- pParse->is_aborted =3D true;
- return;
- }
- strncpy(field->default_value, pSpan->zStart,
- default_length);
- field->default_value[default_length] =3D '\0';
+ goto add_default_value_exit;
}
+ strncpy(field->default_value, pSpan->zStart, =
default_length);
+ field->default_value[default_length] =3D '\0';
}
+add_default_value_exit:
sql_expr_delete(db, pSpan->pExpr, false);
}
>> + field =3D &def->fields[def->field_count - 1];
>> + if (!sqlExprIsConstantOrFunction(pSpan->pExpr, db->init.busy)) {
>> + diag_set(ClientError, ER_CREATE_SPACE, def->name,
>> + tt_sprintf("default value of column '%s' is not =
"
>> + "constant", field->name));
>> + pParse->is_aborted =3D true;
>> + } else {
>> + assert(def !=3D NULL);
>> + struct region *region =3D &pParse->region;
>> + uint32_t default_length =3D (int)(pSpan->zEnd - =
pSpan->zStart);
>> + field->default_value =3D region_alloc(region, =
default_length + 1);
>> + if (field->default_value =3D=3D NULL) {
>> + diag_set(OutOfMemory, default_length + 1,
>> + "region_alloc", =
"field->default_value");
>> pParse->is_aborted =3D true;
>> - } else {
>> - assert(def !=3D NULL);
>> - struct field_def *field =3D
>> - &def->fields[def->field_count - 1];
>> - struct region *region =3D &pParse->region;
>> - uint32_t default_length =3D (int)(pSpan->zEnd - =
pSpan->zStart);
>> - field->default_value =3D region_alloc(region,
>> - =
default_length + 1);
>> - if (field->default_value =3D=3D NULL) {
>> - diag_set(OutOfMemory, default_length + =
1,
>> - "region_alloc",
>> - "field->default_value");
>> - pParse->is_aborted =3D true;
>> - return;
>> - }
>> - strncpy(field->default_value, pSpan->zStart,
>> - default_length);
>> - field->default_value[default_length] =3D '\0';
>> + goto add_default_value_exit;
>> }
>> + strncpy(field->default_value, pSpan->zStart, =
default_length);
>> + field->default_value[default_length] =3D '\0';
>> }
>> +add_default_value_exit:
>> sql_expr_delete(db, pSpan->pExpr, false);
>> }
>>=20
>> @@ -447,6 +587,8 @@ sqlAddPrimaryKey(struct Parse *pParse)
>> int nTerm;
>> struct ExprList *pList =3D pParse->create_index_def.cols;
>> struct space *space =3D pParse->create_table_def.new_space;
>> + if (space =3D=3D NULL)
>> + space =3D pParse->create_column_def.space;
>> if (space =3D=3D NULL)
>> goto primary_key_exit;
>> if (sql_space_primary_key(space) !=3D NULL) {
>> @@ -574,8 +716,10 @@ sql_create_check_contraint(struct Parse *parser)
>> (struct alter_entity_def *) create_ck_def;
>> assert(alter_def->entity_type =3D=3D ENTITY_TYPE_CK);
>> (void) alter_def;
>> - struct space *space =3D parser->create_table_def.new_space;
>> - bool is_alter =3D space =3D=3D NULL;
>> + struct space *space =3D parser->create_column_def.space;
>> + if (space =3D=3D NULL)
>> + space =3D parser->create_table_def.new_space;
>> + bool is_alter_add_constr =3D space =3D=3D NULL;
>=20
> 18. Why is it called 'is_alter' in other functions, but =
'is_alter_add_constr'
> here?
If I added =E2=80=98is_alter_add_constr=E2=80=99, it means that we =
should distinguish current
statements: <ALTER TABLE ADD CONSTRAINT> or <ALTER TABLE ADD COLUMN> =
(maybe <CREATE INDEX>).
It=E2=80=99s important for branching within constraint creation =
functions (index, ck, fk).
>>=20
>> /* Prepare payload for ck constraint definition. */
>> struct region *region =3D &parser->region;
>> @@ -589,9 +733,18 @@ sql_create_check_contraint(struct Parse *parser)
>> return;
>> }
>> } else {
>> - assert(! is_alter);
>> - uint32_t ck_idx =3D =
++parser->create_table_def.check_count;
>> - name =3D tt_sprintf("ck_unnamed_%s_%d", =
space->def->name, ck_idx);
>> + assert(!is_alter_add_constr);
>> + uint32_t idx =3D ++parser->check_count;
>> + if (parser->create_table_def.new_space =3D=3D NULL) {
>> + struct space *original_space =3D
>> + space_by_name(space->def->name);
>> + assert(original_space !=3D NULL);
>> + struct rlist *checks =3D =
&original_space->ck_constraint;
>> + struct ck_constraint *ck;
>> + rlist_foreach_entry(ck, checks, link)
>> + idx++;
>> + }
>> + name =3D tt_sprintf("ck_unnamed_%s_%d", =
space->def->name, idx);
>> }
>> size_t name_len =3D strlen(name);
>>=20
>> @@ -634,9 +787,9 @@ sql_create_check_contraint(struct Parse *parser)
>> trim_space_snprintf(ck_def->expr_str, expr_str, expr_str_len);
>> memcpy(ck_def->name, name, name_len);
>> ck_def->name[name_len] =3D '\0';
>> - if (is_alter) {
>> + if (is_alter_add_constr) {
>> const char *space_name =3D =
alter_def->entity_name->a[0].zName;
>> - struct space *space =3D space_by_name(space_name);
>> + space =3D space_by_name(space_name);
>=20
> 19. Why this change?
Dropped.
>=20
>> if (space =3D=3D NULL) {
>> diag_set(ClientError, ER_NO_SUCH_SPACE, =
space_name);
>> parser->is_aborted =3D true;
>> @@ -652,8 +805,7 @@ sql_create_check_contraint(struct Parse *parser)
>> sqlVdbeCountChanges(v);
>> sqlVdbeChangeP5(v, OPFLAG_NCHANGE);
>> } else {
>> - rlist_add_entry(&parser->create_table_def.new_check, =
ck_parse,
>> - link);
>> + rlist_add_entry(&parser->checks, ck_parse, link);
>> }
>> }
>>=20
>> @@ -664,18 +816,19 @@ sql_create_check_contraint(struct Parse =
*parser)
>> void
>> sqlAddCollateType(Parse * pParse, Token * pToken)
>> {
>> - struct space *space =3D pParse->create_table_def.new_space;
>> - if (space =3D=3D NULL)
>> - return;
>> + struct space *space =3D pParse->create_column_def.space;
>> + uint32_t *coll_id =3D NULL;
>> + assert(space !=3D NULL);
>> uint32_t i =3D space->def->field_count - 1;
>> + coll_id =3D &space->def->fields[i].coll_id;
>=20
> 20. Why not declare and initialize coll_id in one line?
Done.
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);
>> sql *db =3D pParse->db;
>> char *coll_name =3D sql_name_from_token(db, pToken);
>> if (coll_name =3D=3D NULL) {
>> pParse->is_aborted =3D true;
>> return;
>> }
>> - uint32_t *coll_id =3D &space->def->fields[i].coll_id;
>> - if (sql_get_coll_seq(pParse, coll_name, coll_id) !=3D NULL) {
>> + if (sql_get_coll_seq(pParse, coll_name, coll_id) !=3D NULL &&
>> + space !=3D NULL) {
>=20
> 21. You have assert(space !=3D NULL) above. Why do you need this check =
again?
Removed.
>> /* If the column is declared as "<name> PRIMARY KEY =
COLLATE <type>",
>> * then an index may have been created on this column =
before the
>> * collation type was added. Correct this if it is the =
case.
>> @@ -692,6 +845,7 @@ sqlAddCollateType(Parse * pParse, Token * pToken)
>> sqlDbFree(db, coll_name);
>> }
>>=20
>> +
>=20
> 22. ???
Oops. Removed.
>> struct coll *
>> sql_column_collation(struct space_def *def, uint32_t column, uint32_t =
*coll_id)
>> {
>> @@ -1147,6 +1304,105 @@ resolve_link(struct Parse *parse_context, =
const struct space_def *def,
>> return -1;
>> }
>>=20
>> +/**
>> + * Emit code to create sequences, indexes, check and foreign
>> + * constraints appeared in <CREATE TABLE> or
>> + * <ALTER TABLE ADD COLUMN> statement.
>> + */
>> +static void
>> +sql_vdbe_create_constraints(struct Parse *parse, int reg_space_id)
>=20
> 23. Such huge code movements better do in a separate commit. I can't
> tell whether you changed anything here, or just moved the code.
Done. See commit in the new version of the patch set.
>> +{
>> + assert(reg_space_id !=3D 0);
>> + struct space *space =3D parse->create_table_def.new_space;
>> + bool is_alter =3D space =3D=3D NULL;
>> + uint32_t i =3D 0;
>> + if (is_alter) {
>> + space =3D parse->create_column_def.space;
>> + i =3D space_by_name(space->def->name)->index_count;
>> + }
>> + assert(space !=3D NULL);
>> + for (; i < space->index_count; ++i) {
>> + struct index *idx =3D space->index[i];
>> + vdbe_emit_create_index(parse, space->def, idx->def,
>> + reg_space_id, idx->def->iid);
>> + }
>> +
>> + /*
>> + * Check to see if we need to create an _sequence table
>> + * for keeping track of autoincrement keys.
>> + */
>> + if (parse->has_autoinc) {
>> + /* Do an insertion into _sequence. */
>> + int reg_seq_id =3D ++parse->nMem;
>> + struct Vdbe *v =3D sqlGetVdbe(parse);
>> + assert(v !=3D NULL);
>> + sqlVdbeAddOp2(v, OP_NextSequenceId, 0, reg_seq_id);
>> + int reg_seq_record =3D
>> + emitNewSysSequenceRecord(parse, reg_seq_id,
>> + space->def->name);
>> + const char *error_msg =3D
>> + =
tt_sprintf(tnt_errcode_desc(ER_SQL_CANT_ADD_AUTOINC),
>> + space->def->name);
>> + if (vdbe_emit_halt_with_presence_test(parse,
>> + BOX_SEQUENCE_ID, =
2,
>> + reg_seq_record + =
3, 1,
>> + =
ER_SQL_CANT_ADD_AUTOINC,
>> + error_msg, false,
>> + OP_NoConflict) !=3D =
0)
>> + return;
>> + sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, =
reg_seq_record);
>> + /* Do an insertion into _space_sequence. */
>> + int reg_space_seq_record =3D
>> + emitNewSysSpaceSequenceRecord(parse, =
reg_space_id,
>> + reg_seq_id);
>> + sqlVdbeAddOp2(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID,
>> + reg_space_seq_record);
>> + }
>> +
>> + /* Code creation of FK constraints, if any. */
>> + struct fk_constraint_parse *fk_parse;
>> + rlist_foreach_entry(fk_parse, &parse->fkeys, link) {
>> + struct fk_constraint_def *fk_def =3D fk_parse->fk_def;
>> + if (fk_parse->selfref_cols !=3D NULL) {
>> + struct ExprList *cols =3D =
fk_parse->selfref_cols;
>> + for (uint32_t i =3D 0; i < fk_def->field_count; =
++i) {
>> + if (resolve_link(parse, space->def,
>> + cols->a[i].zName,
>> + =
&fk_def->links[i].parent_field,
>> + fk_def->name) !=3D 0)
>> + return;
>> + }
>> + fk_def->parent_id =3D reg_space_id;
>> + } else if (fk_parse->is_self_referenced) {
>> + struct key_def *pk_key_def =3D
>> + =
sql_space_primary_key(space)->def->key_def;
>> + if (pk_key_def->part_count !=3D =
fk_def->field_count) {
>> + diag_set(ClientError, =
ER_CREATE_FK_CONSTRAINT,
>> + fk_def->name, "number of =
columns in "\
>> + "foreign key does not match the =
"\
>> + "number of columns in the =
primary "\
>> + "index of referenced table");
>> + parse->is_aborted =3D true;
>> + return;
>> + }
>> + for (uint32_t i =3D 0; i < fk_def->field_count; =
++i) {
>> + fk_def->links[i].parent_field =3D
>> + pk_key_def->parts[i].fieldno;
>> + }
>> + fk_def->parent_id =3D reg_space_id;
>> + }
>> + fk_def->child_id =3D reg_space_id;
>> + vdbe_emit_fk_constraint_create(parse, fk_def, =
space->def->name);
>> + }
>> +
>> + /* Code creation of CK constraints, if any. */
>> + struct ck_constraint_parse *ck_parse;
>> + rlist_foreach_entry(ck_parse, &parse->checks, link) {
>> + vdbe_emit_ck_constraint_create(parse, ck_parse->ck_def,
>> + reg_space_id, =
space->def->name);
>> + }
>> +}
>> +
>> /*
>> * This routine is called to report the final ")" that terminates
>> * a CREATE TABLE statement.
>> @@ -1213,73 +1469,7 @@ sqlEndTable(struct Parse *pParse)
>>=20
>> int reg_space_id =3D getNewSpaceId(pParse);
>> vdbe_emit_space_create(pParse, reg_space_id, name_reg, =
new_space);
>> - for (uint32_t i =3D 0; i < new_space->index_count; ++i) {
>> - struct index *idx =3D new_space->index[i];
>> - vdbe_emit_create_index(pParse, new_space->def, idx->def,
>> - reg_space_id, idx->def->iid);
>> - }
>> -
>> - /*
>> - * Check to see if we need to create an _sequence table
>> - * for keeping track of autoincrement keys.
>> - */
>> - if (pParse->create_table_def.has_autoinc) {
>> - assert(reg_space_id !=3D 0);
>> - /* Do an insertion into _sequence. */
>> - int reg_seq_id =3D ++pParse->nMem;
>> - sqlVdbeAddOp2(v, OP_NextSequenceId, 0, reg_seq_id);
>> - int reg_seq_record =3D
>> - emitNewSysSequenceRecord(pParse, reg_seq_id,
>> - new_space->def->name);
>> - sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, =
reg_seq_record);
>> - /* Do an insertion into _space_sequence. */
>> - int reg_space_seq_record =3D
>> - emitNewSysSpaceSequenceRecord(pParse, =
reg_space_id,
>> - reg_seq_id);
>> - sqlVdbeAddOp2(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID,
>> - reg_space_seq_record);
>> - }
>> - /* Code creation of FK constraints, if any. */
>> - struct fk_constraint_parse *fk_parse;
>> - rlist_foreach_entry(fk_parse, =
&pParse->create_table_def.new_fkey,
>> - link) {
>> - struct fk_constraint_def *fk_def =3D fk_parse->fk_def;
>> - if (fk_parse->selfref_cols !=3D NULL) {
>> - struct ExprList *cols =3D =
fk_parse->selfref_cols;
>> - for (uint32_t i =3D 0; i < fk_def->field_count; =
++i) {
>> - if (resolve_link(pParse, new_space->def,
>> - cols->a[i].zName,
>> - =
&fk_def->links[i].parent_field,
>> - fk_def->name) !=3D 0)
>> - return;
>> - }
>> - fk_def->parent_id =3D reg_space_id;
>> - } else if (fk_parse->is_self_referenced) {
>> - struct index *pk =3D =
sql_space_primary_key(new_space);
>> - if (pk->def->key_def->part_count !=3D =
fk_def->field_count) {
>> - diag_set(ClientError, =
ER_CREATE_FK_CONSTRAINT,
>> - fk_def->name, "number of =
columns in "\
>> - "foreign key does not match the =
"\
>> - "number of columns in the =
primary "\
>> - "index of referenced table");
>> - pParse->is_aborted =3D true;
>> - return;
>> - }
>> - for (uint32_t i =3D 0; i < fk_def->field_count; =
++i) {
>> - fk_def->links[i].parent_field =3D
>> - =
pk->def->key_def->parts[i].fieldno;
>> - }
>> - fk_def->parent_id =3D reg_space_id;
>> - }
>> - fk_def->child_id =3D reg_space_id;
>> - vdbe_emit_fk_constraint_create(pParse, fk_def, =
space_name_copy);
>> - }
>> - struct ck_constraint_parse *ck_parse;
>> - rlist_foreach_entry(ck_parse, =
&pParse->create_table_def.new_check,
>> - link) {
>> - vdbe_emit_ck_constraint_create(pParse, ck_parse->ck_def,
>> - reg_space_id, =
space_name_copy);
>> - }
>> + sql_vdbe_create_constraints(pParse, reg_space_id);
>> }
>> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
>> index 995875566..5904414a3 100644
>> --- a/src/box/sql/parse.y
>> +++ b/src/box/sql/parse.y
>> @@ -314,6 +320,7 @@ ccons ::=3D cconsname(N) PRIMARY KEY =
sortorder(Z). {
>> create_index_def_init(&pParse->create_index_def, NULL, &N, NULL,
>> SQL_INDEX_TYPE_CONSTRAINT_PK, Z, false);
>> sqlAddPrimaryKey(pParse);
>> + pParse->create_column_def.is_pk =3D true;
>=20
> 24. Why can't this be done in sqlAddPrimaryKey(pParse);? What if this
> is CREATE TABLE, and there are many columns in PK?
I removed this field from create_column_def. It turned out to be =
unnecessary.
>> }
>> ccons ::=3D cconsname(N) UNIQUE. {
>> create_index_def_init(&pParse->create_index_def, NULL, &N, NULL,
>> diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
>> index a5a258805..b31bac437 100644
>> --- a/src/box/sql/prepare.c
>> +++ b/src/box/sql/prepare.c
>> @@ -197,9 +197,13 @@ sql_parser_create(struct Parse *parser, struct =
sql *db, uint32_t sql_flags)
>> {
>> memset(parser, 0, sizeof(struct Parse));
>> parser->db =3D db;
>> + parser->create_column_def.nullable_action =3D =
ON_CONFLICT_ACTION_DEFAULT;
>=20
> 25. Why isn't there a proper constructor for create_column_def =
structure?
Dropped.
>> parser->sql_flags =3D sql_flags;
>> parser->line_count =3D 1;
>> parser->line_pos =3D 1;
>> + rlist_create(&parser->fkeys);
>> + rlist_create(&parser->checks);
> 26. Why did you merge these fields into struct Parse? They were =
specially
> moved out of here for the purpose of some kind of isolation, into =
parse_def.h
> structures.
Because these fields are convenient to use right here, so as not to =
duplicate
the code in build.c. These fields are used by two statements: CREATE =
TABLE and
ALTER TABLE ADD COLUMN.
>> + parser->has_autoinc =3D false;
>> region_create(&parser->region, &cord()->slabc);
>> }
>>=20
>> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
>> index aa6a470f8..3143ec521 100644
>> --- a/src/box/sql/sqlInt.h
>> +++ b/src/box/sql/sqlInt.h
>> @@ -2249,12 +2249,26 @@ struct Parse {
>> struct enable_entity_def enable_entity_def;
>> };
>> /**
>> - * Table def is not part of union since information
>> - * being held must survive till the end of parsing of
>> - * whole CREATE TABLE statement (to pass it to
>> - * sqlEndTable() function).
>> + * Table def or column def is not part of union since
>> + * information being held must survive till the end of
>> + * parsing of whole <CREATE TABLE> or
>> + * <ALTER TABLE ADD COLUMN> 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 <CREATE TABLE> or
>> + * a <ALTER TABLE ADD COLUMN> statement.
>> + */
>> + struct rlist fkeys;
>> + struct rlist checks;
>> + uint32_t fkey_count;
>> + uint32_t check_count;
>> + /** True, if column to be created has <AUTOINCREMENT>. */
>> + bool has_autoinc;
>=20
> 27. What column? This is struct Parse, it is not a column.
I know, but I haven't come up with anything better.
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index fa87e7bd2..32142a871 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2251,22 +2251,26 @@ struct Parse {
struct enable_entity_def enable_entity_def;
};
/**
- * Table def is not part of union since information
- * being held must survive till the end of parsing of
- * whole CREATE TABLE statement (to pass it to
- * sqlEndTable() function).
+ * Table def or column def is not part of union since
+ * information being held must survive till the end of
+ * parsing of whole <CREATE TABLE> or
+ * <ALTER TABLE ADD COLUMN> 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 <CREATE TABLE>.
+ * FK and CK constraints appeared in a <CREATE TABLE> or
+ * an <ALTER TABLE ADD COLUMN> statement.
*/
struct rlist fkeys;
struct rlist checks;
uint32_t fkey_count;
uint32_t check_count;
/*
- * True, if column within a <CREATE TABLE> statement to be
- * created has <AUTOINCREMENT>.
+ * True, if column in a <CREATE TABLE> or an
+ * <ALTER TABLE ADD COLUMN> statement to be created has
+ * <AUTOINCREMENT>.
*/
bool has_autoinc;
/* Id of field with <AUTOINCREMENT>. */
>> + /** Id of field with <AUTOINCREMENT>. */
>> + uint32_t autoinc_fieldno;
>> bool initiateTTrans; /* Initiate Tarantool transaction */
>> /** If set - do not emit byte code at all, just parse. */
>> bool parse_only;
>> @@ -2844,15 +2858,31 @@ struct space *sqlResultSetOfSelect(Parse *, =
Select *);
>>=20
>> struct space *
>> sqlStartTable(Parse *, Token *);
>> -void sqlAddColumn(Parse *, Token *, struct type_def *);
>> +
>> +/**
>> + * Add new field to the format of ephemeral space in
>> + * create_table_def. If it is <ALTER TABLE> create shallow copy of
>> + * the existing space and add field to its format.
>> + */
>> +void
>> +sql_create_column_start(struct Parse *parse);
>> +
>> +/**
>> + * Nullify create_column_def. If it is <ALTER TABLE> 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 <CREATE TABLE> or a <ALTER TABLE ADD COLUMN>
>> + * statement. A "NOT NULL" constraint has been seen on a column.
>> + * This routine sets the is_nullable flag on the column currently
>> + * under construction. If nullable_action has been already set,
>> + * this function raises an error.
>> *
>> * @param parser SQL Parser object.
>> * @param nullable_action on_conflict_action value.
>> diff --git a/test/box/error.result b/test/box/error.result
>> index 2196fa541..eb55f9cdf 100644
>> --- a/test/box/error.result
>> +++ b/test/box/error.result
>> @@ -432,6 +432,8 @@ t;
>> | 211: box.error.WRONG_QUERY_ID
>> | 212: box.error.SEQUENCE_NOT_STARTED
>> | 213: box.error.NO_SUCH_SESSION_SETTING
>> + | 214: box.error.SQL_CANT_ADD_COLUMN_TO_VIEW
>> + | 215: box.error.SQL_CANT_ADD_AUTOINC
>> | ...
>>=20
>> test_run:cmd("setopt delimiter ''");
>> diff --git a/test/sql/add-column.result b/test/sql/add-column.result
>> new file mode 100644
>> index 000000000..06c95facb
>> --- /dev/null
>> +++ b/test/sql/add-column.result
>=20
> 28. You may want to look at the original SQLite tests. They probably
> have some.
I looked. I took everything that is relevant for Tarantool from there.
>> @@ -0,0 +1,276 @@
>> +-- test-run result file version 2
>> +test_run =3D require('test_run').new()
>> + | ---
>> + | ...
>> +engine =3D test_run:get_cfg('engine')
>> + | ---
>> + | ...
>> +_ =3D box.space._session_settings:update('sql_default_engine', =
{{'=3D', 2, engine}})
>> + | ---
>> + | ...
>> +
>> +--
>> +-- gh-3075: Check <ALTER TABLE table ADD COLUMN column> statement.
>> +--
>> +box.execute('CREATE TABLE t1 (a INTEGER PRIMARY KEY);')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +--
>> +-- COLUMN keyword is optional. Check it here, but omit it below.
>> +--
>> +box.execute('ALTER TABLE t1 ADD COLUMN b INTEGER;')
>> + | ---
>> + | - row_count: 0
>> + | ...
>> +
>> +--
>> +-- Can't add column to a view.
>> +--
>> +box.execute('CREATE VIEW v AS SELECT * FROM t1;')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute('ALTER TABLE v ADD b INTEGER;')
>> + | ---
>> + | - null
>> + | - Can't add column 'B'. 'V' is a view
>> + | ...
>> +box.execute('DROP VIEW v;')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +
>> +--
>> +-- Check column constraints typing and work.
>> +--
>> +box.execute('CREATE TABLE t2 (a INTEGER CONSTRAINT pk_constr PRIMARY =
KEY);')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute('ALTER TABLE t2 DROP CONSTRAINT pk_constr')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +test_run:cmd("setopt delimiter ';'");
>> + | ---
>> + | - true
>> + | ...
>> +box.execute([[ALTER TABLE t2 ADD b INTEGER CONSTRAINT pk_constr =
PRIMARY KEY
>> + CHECK (b > 0)
>> + REFERENCES t1(a)
>> + CONSTRAINT u_constr UNIQUE]])
>> +test_run:cmd("setopt delimiter ''");
>> + | ---
>> + | ...
>> +box.execute('INSERT INTO t1 VALUES (1, 1);')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute('INSERT INTO t2 VALUES (1, 1);')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute('INSERT INTO t2 VALUES (1, 1);')
>> + | ---
>> + | - null
>> + | - Duplicate key exists in unique index 'PK_CONSTR' in space 'T2'
>> + | ...
>> +
>> +box.execute('INSERT INTO t1 VALUES (0, 1);')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute('INSERT INTO t2 VALUES (2, 0);')
>> + | ---
>> + | - null
>> + | - 'Check constraint failed ''ck_unnamed_T2_1'': b > 0'
>> + | ...
>> +
>> +box.execute('INSERT INTO t2 VALUES (2, 3);')
>> + | ---
>> + | - null
>> + | - 'Failed to execute SQL statement: FOREIGN KEY constraint =
failed'
>> + | ...
>> +
>> +box.execute('DROP TABLE t2;')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +
>> +--
>> +-- Check self-referenced FK creation.
>> +--
>> +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY);')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute('ALTER TABLE t2 ADD b INT REFERENCES t1')
>> + | ---
>> + | - row_count: 0
>=20
> 29. Worth checking if it works. Not just created and dropped right =
away.
See the new version of tests in the diff.
>> + | ...
>> +
>> +box.execute('DROP TABLE t2;')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +
>> +--
>> +-- Check AUTOINCREMENT work.
>> +--
>> +box.execute("CREATE TABLE t2(a INTEGER CONSTRAINT pk PRIMARY KEY);")
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute("ALTER TABLE t2 DROP CONSTRAINT pk;")
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute("ALTER TABLE t2 ADD b INTEGER PRIMARY KEY =
AUTOINCREMENT;")
>> + | ---
>> + | - row_count: 0
>=20
> 30. The same.
>=20
>> + | ...
>> +box.execute("ALTER TABLE t2 ADD c INTEGER AUTOINCREMENT;")
>> + | ---
>> + | - null
>> + | - 'Can''t add AUTOINCREMENT: the space ''T2'' already has one =
auto-incremented field'
>> + | ...
>> +
>> +box.execute('DROP TABLE t2;')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +
>> +--
>> +-- Check clauses after column typing and work.
>> +--
>> +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY, b INTEGER);')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +test_run:cmd("setopt delimiter ';'");
>> + | ---
>> + | - true
>> + | ...
>> +box.execute([[ALTER TABLE t2 ADD c TEXT NOT NULL DEFAULT ('a')
>> + COLLATE "unicode_ci";]]);
>> + | ---
>> + | - row_count: 0
>> + | ...
>> +test_run:cmd("setopt delimiter ''");
>> + | ---
>> + | - true
>> + | ...
>> +box.execute('INSERT INTO t2(a, b) VALUES (1, 1);')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute('SELECT * FROM t2;')
>> + | ---
>> + | - metadata:
>> + | - name: A
>> + | type: integer
>> + | - name: B
>> + | type: integer
>> + | - name: C
>> + | type: string
>> + | rows:
>> + | - [1, 1, 'a']
>> + | ...
>> +box.execute('INSERT INTO t2 VALUES (2, 2, NULL);')
>> + | ---
>> + | - null
>> + | - 'Failed to execute SQL statement: NOT NULL constraint failed: =
T2.C'
>> + | ...
>> +box.execute('SELECT * FROM t2 WHERE c LIKE \'A\';')
>> + | ---
>> + | - metadata:
>> + | - name: A
>> + | type: integer
>> + | - name: B
>> + | type: integer
>> + | - name: C
>> + | type: string
>> + | rows:
>> + | - [1, 1, 'a']
>> + | ...
>> +
>> +--
>> +-- Try to add to a non-empty space a [non-]nullable field.
>> +--
>> +box.execute('ALTER TABLE t2 ADD d INTEGER;')
>> + | ---
>> + | - null
>> + | - Tuple field count 3 does not match space field count 4
>> + | ...
>> +box.execute('ALTER TABLE t2 ADD d TEXT NOT NULL');
>> + | ---
>> + | - null
>> + | - Tuple field count 3 does not match space field count 4
>> + | ...
>> +box.execute('ALTER TABLE t2 ADD e TEXT NULL');
>> + | ---
>> + | - null
>> + | - Tuple field count 3 does not match space field count 4
>> + | ...
>> +
>> +--
>> +-- Add to a space with no-SQL adjusted or without format.
>> +--
>> +_ =3D box.schema.space.create('WITHOUT')
>> + | ---
>> + | ...
>> +box.execute("ALTER TABLE WITHOUT ADD a INTEGER;")
>=20
> 31. No need to caps table names in SQL.
Fixed.
> 32. Need to check if it actually worked. The same below.
>=20
>> + | ---
>> + | - row_count: 0
>> + | ...
>> +box.execute("DROP TABLE WITHOUT;")
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +
>> +s =3D box.schema.space.create('NOSQL')
>> + | ---
>> + | ...
>> +s:format{{name =3D 'A', type =3D 'unsigned'}}
>> + | ---
>> + | ...
>> +box.execute("ALTER TABLE NOSQL ADD b INTEGER")
>> + | ---
>> + | - row_count: 0
>> + | ...
>> +
>> +box.execute('DROP TABLE t2;')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +--
>> +-- Add multiple columns inside a transaction.
>> +--
>> +box.execute('CREATE TABLE t2 (a INTEGER PRIMARY KEY)')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.begin() =
\
>> +box.execute('ALTER TABLE t2 ADD b INT') =
\
>> +box.execute('ALTER TABLE t2 ADD c INT') =
\
>> +box.commit()
>> + | ---
>> + | ...
>> +
>> +box.execute('INSERT INTO t2 VALUES (1, 1, 1)')
>> + | ---
>> + | - row_count: 1
>> + | ...
>> +box.execute('SELECT * FROM t2;')
>> + | ---
>> + | - metadata:
>> + | - name: A
>> + | type: integer
>> + | - name: B
>> + | type: integer
>> + | - name: C
>> + | type: integer
>> + | rows:
>> + | - [1, 1, 1]
>=20
> 33. What if I add a column with UNIQUE constraint?
Added.
P.S.
You can see that I added tests in checks.test.lua and =
foreign-keys.test.lua.
I tested the naming of these constraints. This naming is (maybe isn=E2=80=99=
t)
temporary, because it allows such a case:
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: 0
...
box.execute("ALTER TABLE check_naming DROP CONSTRAINT =
\"fk_unnamed_CHECK_NAMING_1\"")
---
- row_count: 1
...
box.execute("ALTER TABLE check_naming ADD c INT REFERENCES t1(a);")
---
- null
- FOREIGN KEY constraint 'fk_unnamed_CHECK_NAMING_2' already exists in =
space 'CHECK_NAMING'
...
To avoid such collisions, we need to use a hash table of names. Let=E2=80=99=
s
put it aside for later.
commit d0fb61f0283f64f1253307d38d1c8cfdea4c933b
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date: Thu Jan 2 19:06:14 2020 +0300
sql: support column addition
=20
Enable to add column to existing space with
<ALTER TABLE ADD [COLUMN]> statement. Column definition can be
supplemented with the four types of constraints, <DEFAULT>,
<COLLATE> 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
<ALTER TABLE table_name ADD [COLUMN] column_name column_type ...>
statement. The column definition is the same as in <CREATE TABLE>
statement.
=20
For example:
=20
```
tarantool> box.execute("CREATE TABLE test (a INTEGER PRIMARY KEY)")
---
- row_count: 1
...
=20
tarantool> box.execute([[ALTER TABLE test ADD COLUMN b TEXT
> CHECK =
(LENGTH(b) > 1)
> NOT NULL
> DEFAULT ('aa')
> COLLATE =
"unicode_ci"
> ]])
---
- row_count: 0
...
```
diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index 486b6b30d..dea047241 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -76,7 +76,7 @@ static Keyword aKeywordTable[] =3D {
{ "CHECK", "TK_CHECK", true },
{ "COLLATE", "TK_COLLATE", true },
{ "COLUMN_NAME", "TK_COLUMN_NAME", true },
- { "COLUMN", "TK_STANDARD", true },
+ { "COLUMN", "TK_COLUMN", true },
{ "COMMIT", "TK_COMMIT", true },
{ "CONFLICT", "TK_CONFLICT", false },
{ "CONSTRAINT", "TK_CONSTRAINT", true },
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 3c21375f5..cbcffb3a8 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -271,6 +271,8 @@ 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_COLUMN_TO_VIEW, "Can't add =
column '%s'. '%s' is a view") \
+ /*220 */_(ER_SQL_CANT_ADD_AUTOINC, "Can't add =
AUTOINCREMENT: the space '%s' already has one auto incremented field") \
=20
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 9013bc86f..6f3d2747d 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -285,48 +285,113 @@ 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
+ * <ALTER TABLE ADD COLUMN> 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)
+ return NULL;
+ ret->index_count =3D space->index_count;
+ ret->index_id_max =3D space->index_id_max;
+ uint32_t indexes_sz =3D sizeof(struct index *) * =
(ret->index_count);
+ ret->index =3D (struct index **) malloc(indexes_sz);
+ if (ret->index =3D=3D NULL) {
+ diag_set(OutOfMemory, indexes_sz, "realloc", =
"ret->index");
+ return NULL;
+ }
+ memcpy(ret->index, space->index, indexes_sz);
+ 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");
+ free(ret->index);
+ return NULL;
+ }
+ memcpy(ret->def->fields, space->def->fields, =
fields_size);
+ }
+
+ return ret;
+}
+
void
-sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def)
+sql_create_column_start(struct Parse *parse)
{
- assert(type_def !=3D NULL);
- char *z;
- sql *db =3D pParse->db;
- if (pParse->create_table_def.new_space =3D=3D NULL)
- return;
- struct space_def *def =3D =
pParse->create_table_def.new_space->def;
+ struct create_column_def *create_column_def =3D =
&parse->create_column_def;
+ struct alter_entity_def *alter_entity_def =3D
+ &create_column_def->base.base;
+ assert(alter_entity_def->entity_type =3D=3D ENTITY_TYPE_COLUMN);
+ assert(alter_entity_def->alter_action =3D=3D =
ALTER_ACTION_CREATE);
+ struct space *space =3D parse->create_table_def.new_space;
+ bool is_alter =3D space =3D=3D NULL;
+ struct sql *db =3D parse->db;
+ if (is_alter) {
+ const char *space_name =3D
+ alter_entity_def->entity_name->a[0].zName;
+ space =3D space_by_name(space_name);
+ if (space =3D=3D NULL) {
+ diag_set(ClientError, ER_NO_SUCH_SPACE, =
space_name);
+ goto tnt_error;
+ }
+ space =3D sql_shallow_space_copy(parse, space);
+ if (space =3D=3D NULL)
+ goto tnt_error;
+ }
+ create_column_def->space =3D space;
+ struct space_def *def =3D space->def;
+ assert(def->opts.is_ephemeral);
=20
#if SQL_MAX_COLUMN
if ((int)def->field_count + 1 > db->aLimit[SQL_LIMIT_COLUMN]) {
diag_set(ClientError, ER_SQL_COLUMN_COUNT_MAX, =
def->name,
def->field_count + 1, =
db->aLimit[SQL_LIMIT_COLUMN]);
- pParse->is_aborted =3D true;
- return;
+ goto tnt_error;
}
#endif
+
+ struct region *region =3D &parse->region;
+ struct Token *name =3D &create_column_def->base.name;
+ char *column_name =3D
+ sql_normalized_name_region_new(region, name->z, =
name->n);
+ if (column_name =3D=3D NULL)
+ goto tnt_error;
+
+ if (is_alter && def->opts.is_view) {
+ diag_set(ClientError, ER_SQL_CANT_ADD_COLUMN_TO_VIEW,
+ column_name, def->name);
+ goto tnt_error;
+ }
+
/*
- * As sql_field_retrieve will allocate memory on region
- * ensure that def is also temporal and would be dropped.
+ * Format can be set in Lua, then exact_field_count can be
+ * zero, but field_count is not.
*/
- assert(def->opts.is_ephemeral);
- if (sql_field_retrieve(pParse, def, def->field_count) =3D=3D =
NULL)
+ 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 +399,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 + 4);
+ sqlVdbeAddOp4(v, OP_Blob, table_stmt_sz, tuple_reg + 6,
+ SQL_SUBTYPE_MSGPACK, raw, P4_DYNAMIC);
+ sqlVdbeAddOp3(v, OP_MakeRecord, tuple_reg, box_space_field_MAX,
+ tuple_reg + box_space_field_MAX);
+ sqlVdbeAddOp4(v, OP_IdxReplace, tuple_reg + box_space_field_MAX, =
0, 0,
+ (char *) system_space, P4_SPACEPTR);
+ sql_vdbe_create_constraints(parse, key_reg);
+
+ /*
+ * Clean up array allocated in sql_shallow_space_copy().
+ */
+ free(space->index);
}
=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,51 +497,42 @@ 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 <CREATE TABLE> or a <ALTER TABLE ADD COLUMN>
+ * statement.
*/
void
sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan)
{
sql *db =3D pParse->db;
- struct space *p =3D pParse->create_table_def.new_space;
- if (p !=3D NULL) {
- assert(p->def->opts.is_ephemeral);
- struct space_def *def =3D p->def;
- if (!sqlExprIsConstantOrFunction
- (pSpan->pExpr, db->init.busy)) {
- const char *column_name =3D
- def->fields[def->field_count - 1].name;
- diag_set(ClientError, ER_CREATE_SPACE, =
def->name,
- tt_sprintf("default value of column =
'%s' is "\
- "not constant", =
column_name));
+ assert(pParse->create_column_def.space !=3D NULL);
+ struct space_def *def =3D pParse->create_column_def.space->def;
+ struct field_def *field =3D &def->fields[def->field_count - 1];
+ if (!sqlExprIsConstantOrFunction(pSpan->pExpr, db->init.busy)) {
+ diag_set(ClientError, ER_CREATE_SPACE, def->name,
+ tt_sprintf("default value of column '%s' is not =
"
+ "constant", field->name));
+ pParse->is_aborted =3D true;
+ } else {
+ struct region *region =3D &pParse->region;
+ uint32_t default_length =3D (int)(pSpan->zEnd - =
pSpan->zStart);
+ field->default_value =3D region_alloc(region, =
default_length + 1);
+ if (field->default_value =3D=3D NULL) {
+ diag_set(OutOfMemory, default_length + 1,
+ "region_alloc", =
"field->default_value");
pParse->is_aborted =3D true;
- } else {
- assert(def !=3D NULL);
- struct field_def *field =3D
- &def->fields[def->field_count - 1];
- struct region *region =3D &pParse->region;
- uint32_t default_length =3D (int)(pSpan->zEnd - =
pSpan->zStart);
- field->default_value =3D region_alloc(region,
- =
default_length + 1);
- if (field->default_value =3D=3D NULL) {
- diag_set(OutOfMemory, default_length + =
1,
- "region_alloc",
- "field->default_value");
- pParse->is_aborted =3D true;
- return;
- }
- strncpy(field->default_value, pSpan->zStart,
- default_length);
- field->default_value[default_length] =3D '\0';
+ goto add_default_value_exit;
}
+ strncpy(field->default_value, pSpan->zStart, =
default_length);
+ field->default_value[default_length] =3D '\0';
}
+add_default_value_exit:
sql_expr_delete(db, pSpan->pExpr, false);
}
=20
@@ -447,6 +571,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 +700,10 @@ sql_create_check_contraint(struct Parse *parser)
(struct alter_entity_def *) create_ck_def;
assert(alter_def->entity_type =3D=3D ENTITY_TYPE_CK);
(void) alter_def;
- struct space *space =3D parser->create_table_def.new_space;
- bool is_alter =3D space =3D=3D NULL;
+ struct space *space =3D parser->create_column_def.space;
+ if (space =3D=3D NULL)
+ space =3D parser->create_table_def.new_space;
+ bool is_alter_add_constr =3D space =3D=3D NULL;
=20
/* Prepare payload for ck constraint definition. */
struct region *region =3D &parser->region;
@@ -589,9 +717,23 @@ sql_create_check_contraint(struct Parse *parser)
return;
}
} else {
- assert(! is_alter);
- uint32_t ck_idx =3D ++parser->check_count;
- name =3D tt_sprintf("ck_unnamed_%s_%d", =
space->def->name, ck_idx);
+ assert(!is_alter_add_constr);
+ uint32_t idx =3D ++parser->check_count;
+ /*
+ * If it is <ALTER TABLE ADD COLUMN> 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 rlist *checks =3D =
&original_space->ck_constraint;
+ struct ck_constraint *ck;
+ rlist_foreach_entry(ck, checks, link)
+ idx++;
+ }
+ name =3D tt_sprintf("ck_unnamed_%s_%d", =
space->def->name, idx);
}
size_t name_len =3D strlen(name);
=20
@@ -634,7 +776,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 +805,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);
@@ -704,8 +845,7 @@ sql_column_collation(struct space_def *def, uint32_t =
column, uint32_t *coll_id)
*
* In cases mentioned above collation is fetched by id.
*/
- if (space =3D=3D NULL) {
- assert(def->opts.is_ephemeral);
+ if (def->opts.is_ephemeral) {
assert(column < (uint32_t)def->field_count);
*coll_id =3D def->fields[column].coll_id;
struct coll_id *collation =3D coll_by_id(*coll_id);
@@ -794,7 +934,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 +1173,21 @@ vdbe_emit_fk_constraint_create(struct Parse =
*parse_context,
P4_DYNAMIC);
/*
* In case we are adding FK constraints during execution
- * of <CREATE TABLE ...> statement, we don't have child
- * id, but we know register where it will be stored.
+ * of <CREATE TABLE ...> or <ALER TABLE ADD COLUMN ...>
+ * statement, we don't have child id, but we know register
+ * where it will be stored.
*/
- if (parse_context->create_table_def.new_space !=3D NULL) {
+ bool is_alter_add_constr =3D
+ parse_context->create_table_def.new_space =3D=3D NULL &&
+ parse_context->create_column_def.space =3D=3D NULL;
+ if (!is_alter_add_constr) {
sqlVdbeAddOp2(vdbe, OP_SCopy, fk->child_id,
constr_tuple_reg + 1);
} else {
sqlVdbeAddOp2(vdbe, OP_Integer, fk->child_id,
constr_tuple_reg + 1);
}
- if (parse_context->create_table_def.new_space !=3D NULL &&
- fk_constraint_is_self_referenced(fk)) {
+ if (!is_alter_add_constr && =
fk_constraint_is_self_referenced(fk)) {
sqlVdbeAddOp2(vdbe, OP_SCopy, fk->parent_id,
constr_tuple_reg + 2);
} else {
@@ -1107,7 +1251,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 +1292,21 @@ 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 <CREATE TABLE>.
+ * constraints appeared in <CREATE TABLE> or
+ * <ALTER TABLE ADD COLUMN>.
*/
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 (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 +1325,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 +2038,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 <CREATE TABLE>
+ * processing or shallow copy of space during <ALTER TABLE
+ * ... ADD COLUMN>. NULL for <ALTER TABLE ... ADD
+ * CONSTRAINT> 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 +2077,8 @@ sql_create_foreign_key(struct Parse =
*parse_context)
goto tnt_error;
}
memset(fk_parse, 0, sizeof(*fk_parse));
+ if (parse_context->create_column_def.space !=3D NULL)
+ child_space =3D space;
rlist_add_entry(&parse_context->fkeys, fk_parse, link);
}
struct Token *parent =3D create_fk_def->parent_name;
@@ -1920,28 +2091,45 @@ sql_create_foreign_key(struct Parse =
*parse_context)
* self-referenced, but in this case parent (which is
* also child) table will definitely exist.
*/
- is_self_referenced =3D !is_alter &&
+ is_self_referenced =3D !is_alter_add_constr &&
strcmp(parent_name, space->def->name) =3D=3D =
0;
struct space *parent_space =3D space_by_name(parent_name);
- if (parent_space =3D=3D NULL) {
- if (is_self_referenced) {
- struct fk_constraint_parse *fk =3D
- rlist_first_entry(&parse_context->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,
+ 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->fkey_count);
+ uint32_t idx =3D ++parse_context->fkey_count;
+ /*
+ * If it is <ALTER TABLE ADD COLUMN> 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;
+ if (!rlist_empty(child_fk)) {
+ struct fk_constraint *fk;
+ rlist_foreach_entry(fk, =
child_fk,
+ =
in_child_space)
+ idx++;
+ }
+ }
+ constraint_name =3D sqlMPrintf(db, =
"fk_unnamed_%s_%d",
+ space->def->name, =
idx);
} else {
constraint_name =3D
sql_name_from_token(db, =
&create_def->name);
@@ -2001,7 +2189,8 @@ sql_create_foreign_key(struct Parse =
*parse_context)
}
int actions =3D create_fk_def->actions;
fk_def->field_count =3D child_cols_count;
- fk_def->child_id =3D child_space !=3D NULL ? =
child_space->def->id : 0;
+ fk_def->child_id =3D table_def->new_space =3D=3D NULL ?
+ child_space->def->id : 0;
fk_def->parent_id =3D parent_space !=3D NULL ? =
parent_space->def->id : 0;
fk_def->is_deferred =3D create_constr_def->is_deferred;
fk_def->match =3D (enum fk_constraint_match) =
(create_fk_def->match);
@@ -2021,7 +2210,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);
/*
@@ -2050,12 +2239,13 @@ sql_create_foreign_key(struct Parse =
*parse_context)
memcpy(fk_def->name, constraint_name, name_len);
fk_def->name[name_len] =3D '\0';
/*
- * In case of CREATE TABLE processing, all foreign keys
- * constraints must be created after space itself, so
- * lets delay it until sqlEndTable() call and simply
+ * In case of <CREATE TABLE> or <ALTER TABLE ... ADD
+ * COLUMN> processing, all foreign keys constraints must
+ * be created after space itself, so lets delay it until
+ * sqlEndTable() or sql_add_column_end() call and simply
* maintain list of all FK constraints inside parser.
*/
- if (!is_alter) {
+ if (!is_alter_add_constr) {
struct fk_constraint_parse *fk_parse =3D
rlist_first_entry(&parse_context->fkeys,
struct fk_constraint_parse, =
link);
@@ -2407,7 +2597,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);
@@ -2420,10 +2613,8 @@ sql_create_index(struct Parse *parse) {
}
goto exit_create_index;
}
- } else {
- if (parse->create_table_def.new_space =3D=3D NULL)
- goto exit_create_index;
- space =3D parse->create_table_def.new_space;
+ } else if (space =3D=3D NULL) {
+ goto exit_create_index;
}
struct space_def *def =3D space->def;
=20
@@ -2458,7 +2649,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) {
@@ -2624,7 +2815,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;
@@ -2712,7 +2903,7 @@ sql_create_index(struct Parse *parse) {
sqlVdbeAddOp0(vdbe, OP_Expire);
}
=20
- if (tbl_name !=3D NULL)
+ if (!is_create_table_or_add_col)
goto exit_create_index;
table_add_index(space, index);
index =3D NULL;
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 995875566..0c9887851 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -226,19 +226,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.
@@ -281,9 +286,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 <CREATE TABLE>
+ * or <ALTER TABLE ADD COLUMN> statement.
+ */
carglist ::=3D carglist ccons.
carglist ::=3D .
%type cconsname { struct Token }
@@ -1735,11 +1742,28 @@ alter_table_start(A) ::=3D ALTER TABLE =
fullname(T) . { A =3D T; }
=20
%type alter_add_constraint {struct alter_args}
alter_add_constraint(A) ::=3D alter_table_start(T) ADD CONSTRAINT =
nm(N). {
+ A.table_name =3D T;
+ A.name =3D N;
+ pParse->initiateTTrans =3D true;
+ }
+
+%type alter_add_column {struct alter_args}
+alter_add_column(A) ::=3D alter_table_start(T) ADD column_name(N). {
A.table_name =3D T;
A.name =3D N;
pParse->initiateTTrans =3D true;
}
=20
+column_name(N) ::=3D COLUMN nm(A). { N =3D A; }
+column_name(N) ::=3D nm(A). { N =3D A; }
+
+cmd ::=3D alter_column_def carglist create_column_end.
+
+alter_column_def ::=3D alter_add_column(N) typedef(Y). {
+ create_column_def_init(&pParse->create_column_def, N.table_name, =
&N.name, &Y);
+ sql_create_column_start(pParse);
+}
+
cmd ::=3D alter_add_constraint(N) FOREIGN KEY LP eidlist(FA) RP =
REFERENCES
nm(T) eidlist_opt(TA) matcharg(M) refargs(R) =
defer_subclause_opt(D). {
create_fk_def_init(&pParse->create_fk_def, N.table_name, &N.name, FA, =
&T, TA,
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index 1105fda6e..336914c57 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -35,6 +35,7 @@
#include "box/fk_constraint.h"
#include "box/key_def.h"
#include "box/sql.h"
+#include "box/constraint_id.h"
=20
/**
* This file contains auxiliary structures and functions which
@@ -154,6 +155,7 @@ enum sql_index_type {
=20
enum entity_type {
ENTITY_TYPE_TABLE =3D 0,
+ ENTITY_TYPE_COLUMN,
ENTITY_TYPE_VIEW,
ENTITY_TYPE_INDEX,
ENTITY_TYPE_TRIGGER,
@@ -207,6 +209,14 @@ struct create_table_def {
struct space *new_space;
};
=20
+struct create_column_def {
+ struct create_entity_def base;
+ /** Shallow space_def copy. */
+ struct space *space;
+ /** Column type. */
+ struct type_def *type_def;
+};
+
struct create_view_def {
struct create_entity_def base;
/**
@@ -464,6 +474,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_view_def_init(struct create_view_def *view_def, struct Token =
*name,
struct Token *create, struct ExprList *aliases,
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index fa87e7bd2..32142a871 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2251,22 +2251,26 @@ struct Parse {
struct enable_entity_def enable_entity_def;
};
/**
- * Table def is not part of union since information
- * being held must survive till the end of parsing of
- * whole CREATE TABLE statement (to pass it to
- * sqlEndTable() function).
+ * Table def or column def is not part of union since
+ * information being held must survive till the end of
+ * parsing of whole <CREATE TABLE> or
+ * <ALTER TABLE ADD COLUMN> 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 <CREATE TABLE>.
+ * FK and CK constraints appeared in a <CREATE TABLE> or
+ * an <ALTER TABLE ADD COLUMN> statement.
*/
struct rlist fkeys;
struct rlist checks;
uint32_t fkey_count;
uint32_t check_count;
/*
- * True, if column within a <CREATE TABLE> statement to be
- * created has <AUTOINCREMENT>.
+ * True, if column in a <CREATE TABLE> or an
+ * <ALTER TABLE ADD COLUMN> statement to be created has
+ * <AUTOINCREMENT>.
*/
bool has_autoinc;
/* Id of field with <AUTOINCREMENT>. */
@@ -2860,15 +2864,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_table_def. If it is <ALTER TABLE> 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 <CREATE TABLE> or a <ALTER TABLE ADD COLUMN>
+ * statement. A "NOT NULL" constraint has been seen on a column.
+ * This routine sets the is_nullable flag on the column currently
+ * under construction. If nullable_action has been already set,
+ * this function raises an error.
*
* @param parser SQL Parser object.
* @param nullable_action on_conflict_action value.
diff --git a/test/box/error.result b/test/box/error.result
index cdecdb221..6fc3cb99f 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -437,6 +437,8 @@ t;
| 216: box.error.SYNC_QUORUM_TIMEOUT
| 217: box.error.SYNC_ROLLBACK
| 218: box.error.TUPLE_METADATA_IS_TOO_BIG
+ | 219: box.error.SQL_CANT_ADD_COLUMN_TO_VIEW
+ | 220: 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..f86259105
--- /dev/null
+++ b/test/sql/add-column.result
@@ -0,0 +1,471 @@
+-- test-run result file version 2
+--
+-- gh-3075: Check <ALTER TABLE table ADD COLUMN column> 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: 0
+ | ...
+
+--
+-- 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 b INT;
+ | ---
+ | - null
+ | - Can't add column 'B'. 'V' is a view
+ | ...
+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: 0
+ | ...
+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: 0
+ | ...
+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: 0
+ | ...
+INSERT INTO ck_check VALUES (1, 0);
+ | ---
+ | - null
+ | - 'Check constraint failed ''ck_unnamed_CK_CHECK_1'': b > 0'
+ | ...
+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: 0
+ | ...
+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'
+ | ...
+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: 0
+ | ...
+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: 0
+ | ...
+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: the space ''AUTOINC_CHECK'' already has =
one auto incremented
+ | 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: 0
+ | ...
+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: 0
+ | ...
+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: 0
+ | ...
+INSERT INTO null_check(a) VALUES (1);
+ | ---
+ | - row_count: 1
+ | ...
+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: 0
+ | ...
+INSERT INTO notnull_check(a) VALUES (1);
+ | ---
+ | - null
+ | - 'Failed to execute SQL statement: NOT NULL constraint failed: =
NOTNULL_CHECK.B'
+ | ...
+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: 0
+ | ...
+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: 0
+ | ...
+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..f8ab3f756
--- /dev/null
+++ b/test/sql/add-column.test.sql
@@ -0,0 +1,167 @@
+--
+-- gh-3075: Check <ALTER TABLE table ADD COLUMN column> 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 b INT;
+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);
+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);
+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);
+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);
+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..513ed1b62 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
+-- <ALTER TABLE ADD COLUMN>.
+--
+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: 0
+...
+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
+-- <ALTER TABLE ADD COLUMN>.
+--
+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..de2a0c512 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
+-- <ALTER TABLE ADD COLUMN>.
+--
+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: 0
+...
+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
+-- <ALTER TABLE ADD COLUMN>.
+--
+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')