[Tarantool-patches] [PATCH] sql: support column addition
Roman Khabibov
roman.habibov at tarantool.org
Mon Mar 9 18:08:56 MSK 2020
Vlad, can you take it, please?
> On Jan 31, 2020, at 18:05, Roman Khabibov <roman.habibov at tarantool.org> wrote:
>
> Enable to add column to existing space with
> <ALTER TABLE ADD [COLUMN]> statement. Column definition can be
> supplemented with <DEFAULT>, <COLLATE> clauses and <[NOT] NULL>
> column constraint.
>
> Closes #3075
>
> @TarantoolBot document
> Title: Add columns to existing tables in SQL
> Now, it is possible to add columns to existing empty spaces using
> <ALTER TABLE table_name ADD [COLUMN] column_name column_type ...>
> statement. The column definition is the same as in <CREATE TABLE>
> statement, except that table constraints (PRIMARY KEY, UNIQUE,
> REFERENCES, CHECK) cannot be specified yet.
>
> For example:
>
> tarantool> box.execute([[CREATE TABLE test (
> a INTEGER PRIMARY KEY
> );]])
> ---
> - row_count: 1
> ...
>
> tarantool> box.execute([[ALTER TABLE test ADD COLUMN
> b TEXT
> NOT NULL
> DEFAULT ('a')
> COLLATE "unicode_ci"
> ;]])
> ---
> - row_count: 0
> ...
> ---
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3075-add-column
> Issue: https://github.com/tarantool/tarantool/issues/3075
>
> P.S.: I didn't implement column constraints in ADD COLUMN statement,
> because of the sql_create_index(), sql_create_foreign_key() and
> sql_create_check_contraint() functions. These functions have
> branching inside:
>
> 1) CREATE TABLE (parse->create_table_def.new_space != NULL)
> We work with the ephemeral space. Use its def, indexes array,
> ck and fk rlists for checks, opcode emitting, etc and modify it on
> parsing level.
>
> 2) ALTER TABLE (parse->create_table_def.new_space == NULL)
> We work with existing space = space_by_name(), but don't modify it.
> This space will be modified after parsing by alter.cc triggers.
>
> In the case of
> ALTER TABLE test ADD a INT PRIMARY KEY CONSTRAINT c UNIQUE CONSTRAINT d UNIQUE ...
> we need to modifiy the existing space, for example UNIQUE constraint
> creation/opcode emitting requires new space def with new modified
> fields array and new index_id_max. The same is for REFERENCES
> constraint. With CHECK it seems easier, but I didn't implement it,
> because I want to do patch in the futue: "Enable constraints in ADD COLUMN",
> if it will be needed.
>
> There is obvious way to implement constraints parsing during
> ADD COLUMN. To do partial copy of space = space_by_name() on region,
> to keep it in parse->create_column_>def and to work with it. Then
> we will have to change the current code a bit, but it is necessary
> to write API to create an ephemeral space based on the existing.
>
> I would like to hear your opinion. Maybe I'm generally wrong in
> thinking about all this, and it could be made easier.
>
> P.P.S.: Could you, please, help me with COLUMN keyword support.
> I couldn't add it as I wanted, i.e., the line in parse.y near the
> line 1754:
> column_name(N) ::= COLUMN nm(A). { N = A; }
> because I had:
> /Users/r.khabibov/tarantool/src/box/sql/parse.h:171:9: error: 'TK_COLUMN' macro redefined
> #define TK_COLUMN
>
> I don't understand why it is so? For example, the definition of
> CONSTRAINT keyword in mkkeywordhash.c is the same, but there is no
> error about this #define during compilation.
>
> { "COLUMN", "TK_COLUMN", true },
> { "CONSTRAINT", "TK_CONSTRAINT", true },
>
> extra/mkkeywordhash.c | 6 +-
> src/box/errcode.h | 1 +
> src/box/sql.c | 28 +++++---
> src/box/sql/alter.c | 125 ++++++++++++++++++++++++++++++++++++
> src/box/sql/build.c | 106 ++++++++++++++++++------------
> src/box/sql/parse.y | 59 +++++++++++++++--
> src/box/sql/parse_def.h | 23 +++++++
> src/box/sql/sqlInt.h | 40 ++++++++++++
> src/box/sql/tarantoolInt.h | 6 +-
> test/box/misc.result | 1 +
> test/sql-tap/alter.test.lua | 76 +++++++++++++++++++++-
> 11 files changed, 405 insertions(+), 66 deletions(-)
>
> diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
> index 006285622..51c8cbb18 100644
> --- a/extra/mkkeywordhash.c
> +++ b/extra/mkkeywordhash.c
> @@ -75,11 +75,7 @@ static Keyword aKeywordTable[] = {
> { "CAST", "TK_CAST", false },
> { "CHECK", "TK_CHECK", true },
> { "COLLATE", "TK_COLLATE", true },
> - /* gh-3075: Reserved until ALTER ADD COLUMN is implemeneted.
> - * Move it back to ALTER when done.
> - */
> - /* { "COLUMN", "TK_COLUMNKW", true }, */
> - { "COLUMN", "TK_STANDARD", true },
> + { "COLUMN", "TK_COLUMN", true },
> { "COMMIT", "TK_COMMIT", true },
> { "CONFLICT", "TK_CONFLICT", false },
> { "CONSTRAINT", "TK_CONSTRAINT", true },
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index 3065b1948..e076edf02 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -264,6 +264,7 @@ struct errcode_record {
> /*209 */_(ER_SESSION_SETTING_INVALID_VALUE, "Session setting %s expected a value of type %s") \
> /*210 */_(ER_SQL_PREPARE, "Failed to prepare SQL statement: %s") \
> /*211 */_(ER_WRONG_QUERY_ID, "Prepared statement with id %u does not exist") \
> + /*212 */_(ER_SQL_CANT_ADD_COLUMN_TO_VIEW, "Can't add column '%s' to the view '%s'") \
>
> /*
> * !IMPORTANT! Please follow instructions at start of the file
> diff --git a/src/box/sql.c b/src/box/sql.c
> index 1256df856..7157cabe6 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -923,7 +923,8 @@ cursor_advance(BtCursor *pCur, int *pRes)
> */
>
> char *
> -sql_encode_table(struct region *region, struct space_def *def, uint32_t *size)
> +sql_encode_table_format(struct region *region, struct field_def *fields,
> + uint32_t field_count, uint32_t *size)
> {
> size_t used = region_used(region);
> struct mpstream stream;
> @@ -931,12 +932,11 @@ sql_encode_table(struct region *region, struct space_def *def, uint32_t *size)
> mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
> set_encode_error, &is_error);
>
> - assert(def != NULL);
> - uint32_t field_count = def->field_count;
> + assert(fields != NULL);
> mpstream_encode_array(&stream, field_count);
> for (uint32_t i = 0; i < field_count && !is_error; i++) {
> - uint32_t cid = def->fields[i].coll_id;
> - struct field_def *field = &def->fields[i];
> + struct field_def *field = &fields[i];
> + uint32_t cid = field->coll_id;
> const char *default_str = field->default_value;
> int base_len = 4;
> if (cid != COLL_NONE)
> @@ -947,16 +947,16 @@ sql_encode_table(struct region *region, struct space_def *def, uint32_t *size)
> mpstream_encode_str(&stream, "name");
> mpstream_encode_str(&stream, field->name);
> mpstream_encode_str(&stream, "type");
> - assert(def->fields[i].is_nullable ==
> - action_is_nullable(def->fields[i].nullable_action));
> + assert(field->is_nullable ==
> + action_is_nullable(field->nullable_action));
> mpstream_encode_str(&stream, field_type_strs[field->type]);
> mpstream_encode_str(&stream, "is_nullable");
> - mpstream_encode_bool(&stream, def->fields[i].is_nullable);
> + mpstream_encode_bool(&stream, field->is_nullable);
> mpstream_encode_str(&stream, "nullable_action");
>
> - assert(def->fields[i].nullable_action < on_conflict_action_MAX);
> + assert(field->nullable_action < on_conflict_action_MAX);
> const char *action =
> - on_conflict_action_strs[def->fields[i].nullable_action];
> + on_conflict_action_strs[field->nullable_action];
> mpstream_encode_str(&stream, action);
> if (cid != COLL_NONE) {
> mpstream_encode_str(&stream, "collation");
> @@ -980,6 +980,14 @@ sql_encode_table(struct region *region, struct space_def *def, uint32_t *size)
> return raw;
> }
>
> +char *
> +sql_encode_table(struct region *region, struct space_def *def, uint32_t *size)
> +{
> + assert(def != NULL);
> + return sql_encode_table_format(region, def->fields, def->field_count,
> + size);
> +}
> +
> char *
> sql_encode_table_opts(struct region *region, struct space_def *def,
> uint32_t *size)
> diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
> index 973b420cf..c12d9373a 100644
> --- a/src/box/sql/alter.c
> +++ b/src/box/sql/alter.c
> @@ -36,6 +36,7 @@
> #include "sqlInt.h"
> #include "box/box.h"
> #include "box/schema.h"
> +#include "tarantoolInt.h"
>
> void
> sql_alter_table_rename(struct Parse *parse)
> @@ -209,3 +210,127 @@ rename_trigger(sql *db, char const *sql_stmt, char const *table_name,
> table_name, tname.z + tname.n);
> return new_sql_stmt;
> }
> +
> +void
> +sql_alter_add_column_start(struct Parse *parse)
> +{
> + struct create_column_def *create_column_def = &parse->create_column_def;
> + struct alter_entity_def *alter_entity_def =
> + &create_column_def->base.base;
> + assert(alter_entity_def->entity_type == ENTITY_TYPE_COLUMN);
> + assert(alter_entity_def->alter_action == ALTER_ACTION_CREATE);
> +
> + const char *space_name =
> + alter_entity_def->entity_name->a[0].zName;
> + struct space *space = space_by_name(space_name);
> + struct sql *db = parse->db;
> + if (space == NULL) {
> + diag_set(ClientError, ER_NO_SUCH_SPACE, space_name);
> + goto tnt_error;
> + }
> +
> + struct space_def *def = space->def;
> + create_column_def->space_def = def;
> + if (sql_space_def_check_format(def) != 0)
> + goto tnt_error;
> + uint32_t new_field_count = def->field_count + 1;
> + create_column_def->new_field_count = new_field_count;
> +
> +#if SQL_MAX_COLUMN
> + if ((int)new_field_count > db->aLimit[SQL_LIMIT_COLUMN]) {
> + diag_set(ClientError, ER_SQL_COLUMN_COUNT_MAX, def->name,
> + new_field_count, db->aLimit[SQL_LIMIT_COLUMN]);
> + goto tnt_error;
> + }
> +#endif
> + /*
> + * Create new fields array and add the new column's
> + * field_def to its end. During copying don't make
> + * duplicates of the non-scalar fields (name,
> + * default_value, default_value_expr), because they will
> + * be just copied and encoded to the msgpack array on sql
> + * allocator in sql_alter_add_column_end().
> + */
> + uint32_t size = sizeof(struct field_def) * new_field_count;
> + struct region *region = &parse->region;
> + struct field_def *new_fields = region_alloc(region, size);
> + if (new_fields == NULL) {
> + diag_set(OutOfMemory, size, "region_alloc", "new_fields");
> + goto tnt_error;
> + }
> + memcpy(new_fields, def->fields,
> + sizeof(struct field_def) * def->field_count);
> + create_column_def->new_fields = new_fields;
> + struct field_def *new_column_def = &new_fields[def->field_count];
> +
> + memcpy(new_column_def, &field_def_default, sizeof(struct field_def));
> + struct Token *name = &create_column_def->base.name;
> + new_column_def->name =
> + sql_normalized_name_region_new(region, name->z, name->n);
> + if (new_column_def->name == NULL)
> + goto tnt_error;
> + if (def->opts.is_view) {
> + diag_set(ClientError, ER_SQL_CANT_ADD_COLUMN_TO_VIEW,
> + new_column_def->name, def->name);
> + goto tnt_error;
> + }
> +
> + new_column_def->type = create_column_def->type_def->type;
> +
> +exit_alter_add_column_start:
> + sqlSrcListDelete(db, alter_entity_def->entity_name);
> + return;
> +tnt_error:
> + parse->is_aborted = true;
> + goto exit_alter_add_column_start;
> +}
> +
> +void
> +sql_alter_add_column_end(struct Parse *parse)
> +{
> + struct create_column_def *create_column_def = &parse->create_column_def;
> + uint32_t new_field_count = create_column_def->new_field_count;
> + uint32_t table_stmt_sz = 0;
> + char *table_stmt =
> + sql_encode_table_format(&parse->region,
> + create_column_def->new_fields,
> + new_field_count, &table_stmt_sz);
> + if (table_stmt == NULL) {
> + parse->is_aborted = true;
> + return;
> + }
> + char *raw = sqlDbMallocRaw(parse->db, table_stmt_sz);
> + if (raw == NULL){
> + parse->is_aborted = true;
> + return;
> + }
> + memcpy(raw, table_stmt, table_stmt_sz);
> +
> + struct space *system_space = space_by_id(BOX_SPACE_ID);
> + assert(system_space != NULL);
> + int cursor = parse->nTab++;
> + struct Vdbe *v = sqlGetVdbe(parse);
> + assert(v != NULL);
> + vdbe_emit_open_cursor(parse, cursor, 0, system_space);
> + sqlVdbeChangeP5(v, OPFLAG_SYSTEMSP);
> +
> + int key_reg = sqlGetTempReg(parse);
> + sqlVdbeAddOp2(v, OP_Integer, create_column_def->space_def->id, key_reg);
> + int addr = sqlVdbeAddOp4Int(v, OP_Found, cursor, 0, key_reg, 1);
> + sqlVdbeAddOp2(v, OP_Halt, -1, ON_CONFLICT_ACTION_ABORT);
> + sqlVdbeJumpHere(v, addr);
> +
> + const int system_space_field_count = 7;
> + int tuple_reg = sqlGetTempRange(parse, system_space_field_count + 1);
> + for (int i = 0; i < system_space_field_count - 1; ++i)
> + sqlVdbeAddOp3(v, OP_Column, cursor, i, tuple_reg + i);
> + sqlVdbeAddOp1(v, OP_Close, cursor);
> +
> + sqlVdbeAddOp2(v, OP_Integer, new_field_count, tuple_reg + 4);
> + sqlVdbeAddOp4(v, OP_Blob, table_stmt_sz, tuple_reg + 6,
> + SQL_SUBTYPE_MSGPACK, raw, P4_DYNAMIC);
> + sqlVdbeAddOp3(v, OP_MakeRecord, tuple_reg, system_space_field_count,
> + tuple_reg + system_space_field_count);
> + sqlVdbeAddOp4(v, OP_IdxReplace, tuple_reg + system_space_field_count, 0,
> + 0, (char *)system_space, P4_SPACEPTR);
> +}
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index bc50ecbfa..2f51b55df 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -368,11 +368,21 @@ void
> sql_column_add_nullable_action(struct Parse *parser,
> enum on_conflict_action nullable_action)
> {
> - struct space *space = parser->create_table_def.new_space;
> - if (space == NULL || NEVER(space->def->field_count < 1))
> + struct space_def *def = NULL;
> + struct field_def *field = NULL;
> + if (parser->create_table_def.new_space != NULL) {
> + def = parser->create_table_def.new_space->def;
> + if (NEVER(def->field_count < 1))
> + return;
> + field = &def->fields[def->field_count - 1];
> + } else if (parser->create_column_def.space_def != NULL) {
> + def = parser->create_column_def.space_def;
> + uint32_t field_count =
> + parser->create_column_def.new_field_count;
> + field = &parser->create_column_def.new_fields[field_count - 1];
> + } else {
> return;
> - struct space_def *def = space->def;
> - struct field_def *field = &def->fields[def->field_count - 1];
> + }
> if (field->nullable_action != ON_CONFLICT_ACTION_DEFAULT &&
> nullable_action != field->nullable_action) {
> /* Prevent defining nullable_action many times. */
> @@ -390,51 +400,53 @@ sql_column_add_nullable_action(struct Parse *parser,
> }
>
> /*
> - * The expression is the default value for the most recently added column
> - * of the table currently under construction.
> + * The expression is the default value for the most recently added
> + * column.
> *
> * Default value expressions must be constant. Raise an exception if this
> * is not the case.
> *
> * This routine is called by the parser while in the middle of
> - * parsing a CREATE TABLE statement.
> + * parsing a <CREATE TABLE> or <ALTER TABLE ADD COLUMN> statement.
> */
> void
> sqlAddDefaultValue(Parse * pParse, ExprSpan * pSpan)
> {
> sql *db = pParse->db;
> - struct space *p = pParse->create_table_def.new_space;
> - if (p != NULL) {
> - assert(p->def->opts.is_ephemeral);
> - struct space_def *def = p->def;
> - if (!sqlExprIsConstantOrFunction
> - (pSpan->pExpr, db->init.busy)) {
> - const char *column_name =
> - def->fields[def->field_count - 1].name;
> - diag_set(ClientError, ER_CREATE_SPACE, def->name,
> - tt_sprintf("default value of column '%s' is "\
> - "not constant", column_name));
> + struct space_def *def = NULL;
> + struct field_def *field = NULL;
> + if (pParse->create_table_def.new_space != NULL) {
> + def = pParse->create_table_def.new_space->def;
> + assert(def->opts.is_ephemeral);
> + field = &def->fields[def->field_count - 1];
> + } else if (pParse->create_column_def.space_def != NULL) {
> + def = pParse->create_column_def.space_def;
> + uint32_t field_count =
> + pParse->create_column_def.new_field_count;
> + field = &pParse->create_column_def.new_fields[field_count - 1];
> + } else {
> + goto add_default_value_exit;
> + }
> + if (!sqlExprIsConstantOrFunction(pSpan->pExpr, db->init.busy)) {
> + diag_set(ClientError, ER_CREATE_SPACE, def->name,
> + tt_sprintf("default value of column '%s' is not "
> + "constant", field->name));
> + pParse->is_aborted = true;
> + } else {
> + assert(def != NULL);
> + struct region *region = &pParse->region;
> + uint32_t default_length = (int)(pSpan->zEnd - pSpan->zStart);
> + field->default_value = region_alloc(region, default_length + 1);
> + if (field->default_value == NULL) {
> + diag_set(OutOfMemory, default_length + 1,
> + "region_alloc", "field->default_value");
> pParse->is_aborted = true;
> - } else {
> - assert(def != NULL);
> - struct field_def *field =
> - &def->fields[def->field_count - 1];
> - struct region *region = &pParse->region;
> - uint32_t default_length = (int)(pSpan->zEnd - pSpan->zStart);
> - field->default_value = region_alloc(region,
> - default_length + 1);
> - if (field->default_value == NULL) {
> - diag_set(OutOfMemory, default_length + 1,
> - "region_alloc",
> - "field->default_value");
> - pParse->is_aborted = true;
> - return;
> - }
> - strncpy(field->default_value, pSpan->zStart,
> - default_length);
> - field->default_value[default_length] = '\0';
> + return;
> }
> + strncpy(field->default_value, pSpan->zStart, default_length);
> + field->default_value[default_length] = '\0';
> }
> +add_default_value_exit:
> sql_expr_delete(db, pSpan->pExpr, false);
> }
>
> @@ -686,17 +698,27 @@ void
> sqlAddCollateType(Parse * pParse, Token * pToken)
> {
> struct space *space = pParse->create_table_def.new_space;
> - if (space == NULL)
> + uint32_t *coll_id = NULL;
> + if (space != NULL) {
> + uint32_t i = space->def->field_count - 1;
> + coll_id = &space->def->fields[i].coll_id;
> + } else if (pParse->create_column_def.space_def != NULL) {
> + uint32_t field_count =
> + pParse->create_column_def.new_field_count;
> + struct field_def *field =
> + &pParse->create_column_def.new_fields[field_count - 1];
> + coll_id = &field->coll_id;
> + } else {
> return;
> - uint32_t i = space->def->field_count - 1;
> + }
> sql *db = pParse->db;
> char *coll_name = sql_name_from_token(db, pToken);
> if (coll_name == NULL) {
> pParse->is_aborted = true;
> return;
> }
> - uint32_t *coll_id = &space->def->fields[i].coll_id;
> - if (sql_get_coll_seq(pParse, coll_name, coll_id) != NULL) {
> + if (sql_get_coll_seq(pParse, coll_name, coll_id) != NULL &&
> + space != NULL) {
> /* If the column is declared as "<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.
> @@ -858,7 +880,9 @@ vdbe_emit_space_create(struct Parse *pParse, int space_id_reg,
> if (table_opts_stmt == NULL)
> goto error;
> uint32_t table_stmt_sz = 0;
> - char *table_stmt = sql_encode_table(region, space->def, &table_stmt_sz);
> + char *table_stmt = sql_encode_table_format(region, space->def->fields,
> + space->def->field_count,
> + &table_stmt_sz);
> if (table_stmt == NULL)
> goto error;
> char *raw = sqlDbMallocRaw(pParse->db,
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> index cfe1c0012..2eba72ac3 100644
> --- a/src/box/sql/parse.y
> +++ b/src/box/sql/parse.y
> @@ -289,10 +289,14 @@ carglist ::= .
> %type cconsname { struct Token }
> cconsname(N) ::= CONSTRAINT nm(X). { N = X; }
> cconsname(N) ::= . { N = Token_nil; }
> -ccons ::= DEFAULT term(X). {sqlAddDefaultValue(pParse,&X);}
> -ccons ::= DEFAULT LP expr(X) RP. {sqlAddDefaultValue(pParse,&X);}
> -ccons ::= DEFAULT PLUS term(X). {sqlAddDefaultValue(pParse,&X);}
> -ccons ::= DEFAULT MINUS(A) term(X). {
> +ccons ::= default_clause_1.
> +default_clause_1 ::= DEFAULT term(X). {sqlAddDefaultValue(pParse,&X);}
> +ccons ::= default_clause_2.
> +default_clause_2 ::= DEFAULT LP expr(X) RP. {sqlAddDefaultValue(pParse,&X);}
> +ccons ::= default_clause_3.
> +default_clause_3 ::= DEFAULT PLUS term(X). {sqlAddDefaultValue(pParse,&X);}
> +ccons ::= default_clause_4.
> +default_clause_4 ::= DEFAULT MINUS(A) term(X). {
> ExprSpan v;
> v.pExpr = sqlPExpr(pParse, TK_UMINUS, X.pExpr, 0);
> v.zStart = A.z;
> @@ -303,13 +307,18 @@ ccons ::= DEFAULT MINUS(A) term(X). {
> // In addition to the type name, we also care about the primary key and
> // UNIQUE constraints.
> //
> -ccons ::= NULL onconf(R). {
> +ccons ::= null_cons.
> +null_cons ::= NULL onconf(R). {
> sql_column_add_nullable_action(pParse, ON_CONFLICT_ACTION_NONE);
> /* Trigger nullability mismatch error if required. */
> if (R != ON_CONFLICT_ACTION_ABORT)
> sql_column_add_nullable_action(pParse, R);
> }
> -ccons ::= NOT NULL onconf(R). {sql_column_add_nullable_action(pParse, R);}
> +ccons ::= not_null_cons.
> +not_null_cons ::= NOT NULL onconf(R). {
> + sql_column_add_nullable_action(pParse, R);
> +}
> +
> ccons ::= cconsname(N) PRIMARY KEY sortorder(Z). {
> create_index_def_init(&pParse->create_index_def, NULL, &N, NULL,
> SQL_INDEX_TYPE_CONSTRAINT_PK, Z, false);
> @@ -335,7 +344,8 @@ ccons ::= cconsname(N) REFERENCES nm(T) eidlist_opt(TA) matcharg(M) refargs(R).
> sql_create_foreign_key(pParse);
> }
> ccons ::= defer_subclause(D). {fk_constraint_change_defer_mode(pParse, D);}
> -ccons ::= COLLATE id(C). {sqlAddCollateType(pParse, &C);}
> +ccons ::= collate_clause.
> +collate_clause ::= COLLATE id(C). {sqlAddCollateType(pParse, &C);}
>
> // The optional AUTOINCREMENT keyword
> %type autoinc {int}
> @@ -1729,11 +1739,46 @@ alter_table_start(A) ::= ALTER TABLE fullname(T) . { A = T; }
>
> %type alter_add_constraint {struct alter_args}
> alter_add_constraint(A) ::= alter_table_start(T) ADD CONSTRAINT nm(N). {
> + A.table_name = T;
> + A.name = N;
> + pParse->initiateTTrans = true;
> + }
> +
> +%type alter_add_column {struct alter_args}
> +alter_add_column(A) ::= alter_table_start(T) ADD column_name(N). {
> A.table_name = T;
> A.name = N;
> pParse->initiateTTrans = true;
> }
>
> +column_name(N) ::= nm(A). { N = A; }
> +
> +cmd ::= alter_column_def alter_carglist add_column_end.
> +
> +alter_column_def ::= alter_add_column(N) typedef(Y). {
> + create_column_def_init(&pParse->create_column_def, N.table_name, &N.name, &Y);
> + sql_alter_add_column_start(pParse);
> +}
> +
> +/*
> + * "alter_carglist" is a list of additional constraints and
> + * clauses that come after the column name and column type in a
> + * ALTER TABLE ADD [COLUMN] statement.
> + */
> +alter_carglist ::= alter_carglist alter_ccons.
> +alter_carglist ::= .
> +alter_ccons ::= default_clause_1.
> +alter_ccons ::= default_clause_2.
> +alter_ccons ::= default_clause_3.
> +alter_ccons ::= default_clause_4.
> +alter_ccons ::= null_cons.
> +alter_ccons ::= not_null_cons.
> +alter_ccons ::= collate_clause.
> +
> +add_column_end ::= . {
> + sql_alter_add_column_end(pParse);
> +}
> +
> cmd ::= alter_add_constraint(N) FOREIGN KEY LP eidlist(FA) RP REFERENCES
> nm(T) eidlist_opt(TA) matcharg(M) refargs(R) defer_subclause_opt(D). {
> create_fk_def_init(&pParse->create_fk_def, N.table_name, &N.name, FA, &T, TA,
> diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
> index 2f433e4c0..56887efa7 100644
> --- a/src/box/sql/parse_def.h
> +++ b/src/box/sql/parse_def.h
> @@ -154,6 +154,7 @@ enum sql_index_type {
>
> enum entity_type {
> ENTITY_TYPE_TABLE = 0,
> + ENTITY_TYPE_COLUMN,
> ENTITY_TYPE_VIEW,
> ENTITY_TYPE_INDEX,
> ENTITY_TYPE_TRIGGER,
> @@ -227,6 +228,18 @@ struct create_table_def {
> uint32_t autoinc_fieldno;
> };
>
> +struct create_column_def {
> + struct create_entity_def base;
> + /** Column type. */
> + struct type_def *type_def;
> + /** Already existing space def. */
> + struct space_def *space_def;
> + /** New format array with the new column. */
> + struct field_def *new_fields;
> + /** Old field count + 1. */
> + uint32_t new_field_count;
> +};
> +
> struct create_view_def {
> struct create_entity_def base;
> /**
> @@ -486,6 +499,16 @@ create_table_def_init(struct create_table_def *table_def, struct Token *name,
> table_def->autoinc_fieldno = 0;
> }
>
> +static inline void
> +create_column_def_init(struct create_column_def *column_def,
> + struct SrcList *table_name, struct Token *name,
> + struct type_def *type_def)
> +{
> + create_entity_def_init(&column_def->base, ENTITY_TYPE_COLUMN,
> + table_name, name, false);
> + column_def->type_def = type_def;
> +}
> +
> static inline void
> create_view_def_init(struct create_view_def *view_def, struct Token *name,
> struct Token *create, struct ExprList *aliases,
> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index d1fcf4761..46d3d6d2a 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -2231,6 +2231,7 @@ struct Parse {
> * from parse.y
> */
> union {
> + struct create_column_def create_column_def;
> struct create_ck_def create_ck_def;
> struct create_fk_def create_fk_def;
> struct create_index_def create_index_def;
> @@ -2842,6 +2843,18 @@ struct space *
> sqlStartTable(Parse *, Token *);
> void sqlAddColumn(Parse *, Token *, struct type_def *);
>
> +/**
> + * Set the is_nullable flag on the column @a field.
> + *
> + * @param field Column to set.
> + * @param nullable_action on_conflict_action value.
> + * @param space_name Name of the space to which the column
> + * belongs. Needed to set error.
> + *
> + * @retval 0 Success.
> + * @retval -1 nullable_action has been already set.
> + */
> +
> /**
> * This routine is called by the parser while in the middle of
> * parsing a CREATE TABLE statement. A "NOT NULL" constraint has
> @@ -2868,6 +2881,19 @@ sqlAddPrimaryKey(struct Parse *parse);
> void
> sql_create_check_contraint(Parse *parser);
>
> +/**
> + * Add default value to the column @a field.
> + *
> + * @param parse Parsing context.
> + * @param field Column to set.
> + * @param span Default expression.
> + * @param space_name Name of the space to which the column
> + * belongs. Needed to set error.
> + *
> + * @retval 0 Success.
> + * @retval -1 Error.
> + */
> +
> void sqlAddDefaultValue(Parse *, ExprSpan *);
> void sqlAddCollateType(Parse *, Token *);
>
> @@ -4031,6 +4057,20 @@ sql_alter_table_rename(struct Parse *parse);
> void
> sql_alter_ck_constraint_enable(struct Parse *parse);
>
> +/**
> + * Process the statement: do the checks of space and column
> + * definition. Create new format array with a new column.
> + */
> +void
> +sql_alter_add_column_start(struct Parse *parse);
> +
> +/**
> + * Encode new format array and emit code to update an entry in
> + * _space.
> + */
> +void
> +sql_alter_add_column_end(struct Parse *parse);
> +
> /**
> * Return the length (in bytes) of the token that begins at z[0].
> * Store the token type in *type before returning.
> diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
> index 1ded6c709..5fd433c60 100644
> --- a/src/box/sql/tarantoolInt.h
> +++ b/src/box/sql/tarantoolInt.h
> @@ -118,14 +118,16 @@ tarantoolsqlIncrementMaxid(uint64_t *space_max_id);
> /**
> * Encode format as entry to be inserted to _space on @region.
> * @param region Region to use.
> - * @param def Space definition to encode.
> + * @param fields Space format.
> + * @param field_count Number of fields.
> * @param[out] size Size of result allocation.
> *
> * @retval NULL Error.
> * @retval not NULL Pointer to msgpack on success.
> */
> char *
> -sql_encode_table(struct region *region, struct space_def *def, uint32_t *size);
> +sql_encode_table_format(struct region *region, struct field_def *fields,
> + uint32_t field_count, uint32_t *size);
>
> /**
> * Encode "opts" dictionary for _space entry on @region.
> diff --git a/test/box/misc.result b/test/box/misc.result
> index 3ca4e31ac..2aaec6566 100644
> --- a/test/box/misc.result
> +++ b/test/box/misc.result
> @@ -563,6 +563,7 @@ t;
> 209: box.error.SESSION_SETTING_INVALID_VALUE
> 210: box.error.SQL_PREPARE
> 211: box.error.WRONG_QUERY_ID
> + 212: box.error.SQL_CANT_ADD_COLUMN_TO_VIEW
> ...
> test_run:cmd("setopt delimiter ''");
> ---
> diff --git a/test/sql-tap/alter.test.lua b/test/sql-tap/alter.test.lua
> index 615b9d8a6..283dc1759 100755
> --- a/test/sql-tap/alter.test.lua
> +++ b/test/sql-tap/alter.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> test = require("sqltester")
> -test:plan(50)
> +test:plan(57)
>
> test:do_execsql_test(
> "alter-1.1",
> @@ -585,4 +585,78 @@ test:do_catchsql_test(
> -- -- </alter-7.11>
> -- })
>
> +--
> +-- gh-3075: Check <ALTER TABLE table ADD COLUMN column> statement.
> +--
> +test:do_execsql_test(
> + "alter-9.1",
> + [[
> + CREATE TABLE test (a INTEGER PRIMARY KEY);
> + ALTER TABLE test ADD b INTEGER;
> + ]], {
> + -- <alter-9.1>
> + -- </alter-9.1>
> + })
> +
> +test:do_catchsql_test(
> + "alter-9.2",
> + [[
> + CREATE VIEW v AS SELECT * FROM test;
> + ALTER TABLE v ADD b INTEGER;
> + ]], {
> + -- <alter-9.2>
> + 1,"Can't add column 'B' to the view 'V'"
> + -- </alter-9.2>
> + })
> +
> +test:do_execsql_test(
> + "alter-9.3",
> + [[
> + ALTER TABLE test ADD c TEXT NOT NULL DEFAULT ('a') COLLATE "unicode_ci";
> + ]], {
> + -- <alter-9.3>
> + -- </alter-9.3>
> + })
> +
> +test:do_execsql_test(
> + "alter-9.4",
> + [[
> + INSERT INTO test(a, b) VALUES (1, 1);
> + SELECT * FROM test;
> + ]], {
> + -- <alter-9.4>
> + 1,1,"a"
> + -- </alter-9.4>
> + })
> +
> +test:do_catchsql_test(
> + "alter-9.5",
> + [[
> + INSERT INTO test VALUES (2, 2, NULL);
> + ]], {
> + -- <alter-9.5>
> + 1,"Failed to execute SQL statement: NOT NULL constraint failed: TEST.C"
> + -- </alter-9.5>
> + })
> +
> +test:do_execsql_test(
> + "alter-9.6",
> + [[
> + SELECT * FROM test WHERE c LIKE 'A';
> + ]], {
> + -- <alter-9.6>
> + 1,1,"a"
> + -- </alter-9.6>
> + })
> +
> +test:do_catchsql_test(
> + "alter-9.7",
> + [[
> + ALTER TABLE test ADD d INTEGER;
> + ]], {
> + -- <alter-9.7>
> + 1,"Tuple field count 3 does not match space field count 4"
> + -- </alter-9.7>
> + })
> +
> test:finish_test()
> --
> 2.21.0 (Apple Git-122)
>
More information about the Tarantool-patches
mailing list