From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v2 1/5] sql: introduce structs assembling DDL arguments during parsing
Date: Fri, 8 Feb 2019 17:25:44 +0300 [thread overview]
Message-ID: <CC4E5BAB-1117-43AB-AC4A-3BC91E706294@tarantool.org> (raw)
In-Reply-To: <6e4d0aae-5dbe-f8d6-3bfe-7723ea2f3d9d@tarantool.org>
Please note, that I failed gracefully separate diffs for
some of comments, so there can be some mess.
I guess it would be better to look at entire diff,
which is attached at the end of letter.
Moreover, I didn’t fix several of your comments completely.
For instance, there are still both ways of getting base structs:
cast and .base invocation. I left my thought on this subject,
but if you feel that it anyway should be fixed, I will do it.
Patch itself is still located at np/gh-3914-fix-create-index-v2
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index f92f39d8e..b6f099cf5 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -383,18 +383,16 @@ sql_table_new(Parse *parser, char *name)
>> * when the "TEMP" or "TEMPORARY" keyword occurs in between
>> * CREATE and TABLE.
>> *
>> - * The new table record is initialized and put in pParse->pNewTable.
>> + * The new table record is initialized and put in pParse->new_table.
>
> 1. No member new_table in struct Parse.
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index b6f099cf5..80fc16af7 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -383,7 +383,7 @@ sql_table_new(Parse *parser, char *name)
* when the "TEMP" or "TEMPORARY" keyword occurs in between
* CREATE and TABLE.
*
- * The new table record is initialized and put in pParse->new_table.
+ * The new table record is initialized and put in pParse->create_table_def.
* As more of the CREATE TABLE statement is parsed, additional action
* routines will be called to add more information to this record.
* At the end of the CREATE TABLE statement, the sqlite3EndTable() routine
>> * As more of the CREATE TABLE statement is parsed, additional action
>> * routines will be called to add more information to this record.
>> * At the end of the CREATE TABLE statement, the sqlite3EndTable() routine
>> * is called to complete the construction of the new table record.
>> *
>> * @param pParse Parser context.
>> - * @param pName1 First part of the name of the table or view.
>> - * @param noErr Do nothing if table already exists.
>> */
>> void
>> -sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
>> +sqlite3StartTable(struct Parse *pParse)
>> {
>> Table *pTable;
>> char *zName = 0; /* The name of the new table */
>> @@ -403,8 +401,9 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
>> if (v == NULL)
>> goto cleanup;
>> sqlite3VdbeCountChanges(v);
>> -
>> - zName = sqlite3NameFromToken(db, pName);
>> + struct Token name =
>> + ((struct create_entity_def *) &pParse->create_table_def)->name;
>> + zName = sqlite3NameFromToken(db, &name);
>
> 2. Looks shorter, no?
>
> if (v == NULL)
> goto cleanup;
> sqlite3VdbeCountChanges(v);
> - struct Token name =
> - ((struct create_entity_def *) &pParse->create_table_def)->name;
> + struct Token name = pParse->create_table_def.base.name;
> zName = sqlite3NameFromToken(db, &name);
> if (zName == 0)
Probably. Applied.
>> if (zName == 0)
>> return;
>> @@ -1788,12 +1795,14 @@ columnno_by_name(struct Parse *parse_context, const struct space *space,
>> }
>> void
>> -sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child,
>> - struct Token *constraint, struct ExprList *child_cols,
>> - struct Token *parent, struct ExprList *parent_cols,
>> - bool is_deferred, int actions)
>> +sql_create_foreign_key(struct Parse *parse_context)
>> {
>> struct sqlite3 *db = parse_context->db;
>> + struct create_fk_def create_fk_def = parse_context->create_fk_def;
>> + struct create_constraint_def create_constr_def = create_fk_def.base;
>> + struct create_entity_def create_def = create_constr_def.base;
>> + struct alter_entity_def alter_def = create_def.base;
>
> 3. Please, do not copy these structs onto the stack. They are too big.
> The same in all other places.
Ok:
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index b6f099cf5..034f811c1 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1798,11 +1797,11 @@ void
sql_create_foreign_key(struct Parse *parse_context)
{
struct sqlite3 *db = parse_context->db;
- struct create_fk_def create_fk_def = parse_context->create_fk_def;
- struct create_constraint_def create_constr_def = create_fk_def.base;
- struct create_entity_def create_def = create_constr_def.base;
- struct alter_entity_def alter_def = create_def.base;
- assert(alter_def.entity_type == ENTITY_TYPE_FK);
+ struct create_fk_def *create_fk_def = &parse_context->create_fk_def;
+ struct create_constraint_def *create_constr_def = &create_fk_def->base;
+ struct create_entity_def *create_def = &create_constr_def->base;
+ struct alter_entity_def *alter_def = &create_def->base;
+ assert(alter_def->entity_type == ENTITY_TYPE_FK);
/*
* When this function is called second time during
* <CREATE TABLE ...> statement (i.e. at VDBE runtime),
@@ -1826,7 +1825,7 @@ sql_create_foreign_key(struct Parse *parse_context)
/* Whether we are processing ALTER TABLE or CREATE TABLE. */
bool is_alter = new_tab == NULL;
uint32_t child_cols_count;
- struct ExprList *child_cols = create_fk_def.child_cols;
+ struct ExprList *child_cols = create_fk_def->child_cols;
if (child_cols == NULL) {
assert(!is_alter);
child_cols_count = 1;
@@ -1835,7 +1834,7 @@ sql_create_foreign_key(struct Parse *parse_context)
}
struct space *child_space = NULL;
if (is_alter) {
- const char *child_name = alter_def.entity_name->a[0].zName;
+ const char *child_name = alter_def->entity_name->a[0].zName;
child_space = space_by_name(child_name);
if (child_space == NULL) {
diag_set(ClientError, ER_NO_SUCH_SPACE, child_name);
@@ -1852,7 +1851,7 @@ sql_create_foreign_key(struct Parse *parse_context)
memset(fk, 0, sizeof(*fk));
rlist_add_entry(&table_def->new_fkey, fk, link);
}
- struct Token *parent = create_fk_def.parent_name;
+ struct Token *parent = create_fk_def->parent_name;
assert(parent != NULL);
parent_name = sqlite3NameFromToken(db, parent);
if (parent_name == NULL)
@@ -1864,7 +1863,7 @@ sql_create_foreign_key(struct Parse *parse_context)
*/
is_self_referenced = !is_alter &&
strcmp(parent_name, new_tab->def->name) == 0;
- struct ExprList *parent_cols = create_fk_def.parent_cols;
+ struct ExprList *parent_cols = create_fk_def->parent_cols;
struct space *parent_space = space_by_name(parent_name);
if (parent_space == NULL) {
if (is_self_referenced) {
@@ -1885,18 +1884,18 @@ sql_create_foreign_key(struct Parse *parse_context)
}
}
if (!is_alter) {
- if (create_def.name.n == 0) {
+ if (create_def->name.n == 0) {
constraint_name =
sqlite3MPrintf(db, "FK_CONSTRAINT_%d_%s",
++table_def->fkey_count,
new_tab->def->name);
} else {
constraint_name =
- sqlite3NameFromToken(db, &create_def.name);
+ sqlite3NameFromToken(db, &create_def->name);
}
} else {
constraint_name =
- sqlite3NameFromToken(db, &create_def.name);
+ sqlite3NameFromToken(db, &create_def->name);
}
if (constraint_name == NULL)
goto exit_create_fk;
@@ -1929,11 +1928,11 @@ sql_create_foreign_key(struct Parse *parse_context)
diag_set(OutOfMemory, fk_size, "region", "struct fkey");
goto tnt_error;
}
- int actions = create_fk_def.actions;
+ int actions = create_fk_def->actions;
fk->field_count = child_cols_count;
fk->child_id = child_space != NULL ? child_space->def->id : 0;
fk->parent_id = parent_space != NULL ? parent_space->def->id : 0;
- fk->is_deferred = create_constr_def.is_deferred;
+ fk->is_deferred = create_constr_def->is_deferred;
fk->match = (enum fkey_match) ((actions >> 16) & 0xff);
fk->on_update = (enum fkey_action) ((actions >> 8) & 0xff);
fk->on_delete = (enum fkey_action) (actions & 0xff);
@@ -2021,9 +2020,9 @@ fkey_change_defer_mode(struct Parse *parse_context, bool is_deferred)
void
sql_drop_foreign_key(struct Parse *parse_context)
{
- struct drop_entity_def drop_def = parse_context->drop_entity_def;
- assert(drop_def.base.entity_type == ENTITY_TYPE_FK);
- const char *table_name = drop_def.base.entity_name->a[0].zName;
+ struct drop_entity_def *drop_def = &parse_context->drop_entity_def;
+ assert(drop_def->base.entity_type == ENTITY_TYPE_FK);
+ const char *table_name = drop_def->base.entity_name->a[0].zName;
assert(table_name != NULL);
struct space *child = space_by_name(table_name);
if (child == NULL) {
@@ -2033,7 +2032,7 @@ sql_drop_foreign_key(struct Parse *parse_context)
return;
}
char *constraint_name = sqlite3NameFromToken(parse_context->db,
- &drop_def.name);
+ &drop_def->name);
if (constraint_name != NULL)
vdbe_emit_fkey_drop(parse_context, constraint_name,
child->def->id);
@@ -2234,22 +2233,22 @@ sql_create_index(struct Parse *parse) {
char *name = NULL;
struct sqlite3 *db = parse->db;
assert(!db->init.busy);
- struct create_index_def create_idx_def = parse->create_index_def;
+ struct create_index_def *create_idx_def = &parse->create_index_def;
struct create_entity_def *create_entity_def =
(struct create_entity_def *) &create_idx_def;
- struct alter_entity_def alter_entity_def = create_entity_def->base;
- assert(alter_entity_def.entity_type == ENTITY_TYPE_INDEX);
+ struct alter_entity_def *alter_entity_def = &create_entity_def->base;
+ assert(alter_entity_def->entity_type == ENTITY_TYPE_INDEX);
/*
* Get list of columns to be indexed. It will be NULL if
* this is a primary key or unique-constraint on the most
* recent column added to the table under construction.
*/
- struct ExprList *col_list = create_idx_def.cols;
- struct SrcList *tbl_name = alter_entity_def.entity_name;
+ struct ExprList *col_list = create_idx_def->cols;
+ struct SrcList *tbl_name = alter_entity_def->entity_name;
if (db->mallocFailed || parse->nErr > 0)
goto exit_create_index;
- enum sql_index_type idx_type = create_idx_def.idx_type;
+ enum sql_index_type idx_type = create_idx_def->idx_type;
if (idx_type == SQL_INDEX_TYPE_UNIQUE ||
idx_type == SQL_INDEX_TYPE_NON_UNIQUE) {
Vdbe *v = sqlite3GetVdbe(parse);
@@ -2386,7 +2385,7 @@ sql_create_index(struct Parse *parse) {
goto exit_create_index;
assert(col_list->nExpr == 1);
sqlite3ExprListSetSortOrder(col_list,
- create_idx_def.sort_order);
+ create_idx_def->sort_order);
} else {
sqlite3ExprListCheckLength(parse, col_list, "index");
}
@@ -2559,14 +2558,14 @@ sql_create_index(struct Parse *parse) {
void
sql_drop_index(struct Parse *parse_context)
{
- struct drop_entity_def drop_def = parse_context->drop_entity_def;
- assert(drop_def.base.entity_type == ENTITY_TYPE_INDEX);
+ struct drop_entity_def *drop_def = &parse_context->drop_entity_def;
+ assert(drop_def->base.entity_type == ENTITY_TYPE_INDEX);
struct Vdbe *v = sqlite3GetVdbe(parse_context);
assert(v != NULL);
struct sqlite3 *db = parse_context->db;
/* Never called with prior errors. */
assert(parse_context->nErr == 0);
- struct SrcList *table_list = drop_def.base.entity_name;
+ struct SrcList *table_list = drop_def->base.entity_name;
assert(table_list->nSrc == 1);
char *table_name = table_list->a[0].zName;
const char *index_name = NULL;
@@ -2575,14 +2574,14 @@ sql_drop_index(struct Parse *parse_context)
}
sqlite3VdbeCountChanges(v);
struct space *space = space_by_name(table_name);
- bool if_exists = drop_def.if_exist;
+ bool if_exists = drop_def->if_exist;
if (space == NULL) {
if (!if_exists)
sqlite3ErrorMsg(parse_context, "no such space: %s",
table_name);
goto exit_drop_index;
}
- index_name = sqlite3NameFromToken(db, &drop_def.name);
+ index_name = sqlite3NameFromToken(db, &drop_def->name);
uint32_t index_id = box_index_id_by_name(space->def->id, index_name,
strlen(index_name));
if (index_id == BOX_ID_NIL) {
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 33b5ea7c4..160b3f56c 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -68,24 +68,24 @@ sql_trigger_begin(struct Parse *parse)
struct sql_trigger *trigger = NULL;
/* The database connection. */
struct sqlite3 *db = parse->db;
- struct create_trigger_def trigger_def = parse->create_trigger_def;
- struct create_entity_def create_def = trigger_def.base;
- struct alter_entity_def alter_def = create_def.base;
- assert(alter_def.entity_type == ENTITY_TYPE_TRIGGER);
+ struct create_trigger_def *trigger_def = &parse->create_trigger_def;
+ struct create_entity_def *create_def = &trigger_def->base;
+ struct alter_entity_def *alter_def = &create_def->base;
+ assert(alter_def->entity_type == ENTITY_TYPE_TRIGGER);
char *trigger_name = NULL;
- if (alter_def.entity_name == NULL || db->mallocFailed)
+ if (alter_def->entity_name == NULL || db->mallocFailed)
goto trigger_cleanup;
- assert(alter_def.entity_name->nSrc == 1);
- assert(create_def.name.n > 0);
- trigger_name = sqlite3NameFromToken(db, &create_def.name);
+ assert(alter_def->entity_name->nSrc == 1);
+ assert(create_def->name.n > 0);
+ trigger_name = sqlite3NameFromToken(db, &create_def->name);
if (trigger_name == NULL)
goto trigger_cleanup;
if (sqlite3CheckIdentifierName(parse, trigger_name) != SQLITE_OK)
goto trigger_cleanup;
- const char *table_name = alter_def.entity_name->a[0].zName;
+ const char *table_name = alter_def->entity_name->a[0].zName;
uint32_t space_id;
if (schema_find_id(BOX_SPACE_ID, 2, table_name, strlen(table_name),
&space_id) != 0)
@@ -108,7 +108,7 @@ sql_trigger_begin(struct Parse *parse)
int name_reg = ++parse->nMem;
sqlite3VdbeAddOp4(parse->pVdbe, OP_String8, 0, name_reg, 0,
name_copy, P4_DYNAMIC);
- bool no_err = create_def.if_not_exist;
+ bool no_err = create_def->if_not_exist;
if (vdbe_emit_halt_with_presence_test(parse, BOX_TRIGGER_ID, 0,
name_reg, 1,
ER_TRIGGER_EXISTS,
@@ -126,12 +126,12 @@ sql_trigger_begin(struct Parse *parse)
trigger->space_id = space_id;
trigger->zName = trigger_name;
trigger_name = NULL;
- assert(trigger_def.op == TK_INSERT || trigger_def.op == TK_UPDATE ||
- trigger_def.op== TK_DELETE);
- trigger->op = (u8) trigger_def.op;
- trigger->tr_tm = trigger_def.tr_tm;
- trigger->pWhen = sqlite3ExprDup(db, trigger_def.when, EXPRDUP_REDUCE);
- trigger->pColumns = sqlite3IdListDup(db, trigger_def.cols);
+ assert(trigger_def->op == TK_INSERT || trigger_def->op == TK_UPDATE ||
+ trigger_def->op== TK_DELETE);
+ trigger->op = (u8) trigger_def->op;
+ trigger->tr_tm = trigger_def->tr_tm;
+ trigger->pWhen = sqlite3ExprDup(db, trigger_def->when, EXPRDUP_REDUCE);
+ trigger->pColumns = sqlite3IdListDup(db, trigger_def->cols);
if ((trigger->pWhen != NULL && trigger->pWhen == NULL) ||
(trigger->pColumns != NULL && trigger->pColumns == NULL))
goto trigger_cleanup;
@@ -141,9 +141,9 @@ sql_trigger_begin(struct Parse *parse)
trigger_cleanup:
sqlite3DbFree(db, trigger_name);
- sqlite3SrcListDelete(db, alter_def.entity_name);
- sqlite3IdListDelete(db, trigger_def.cols);
- sql_expr_delete(db, trigger_def.when, false);
+ sqlite3SrcListDelete(db, alter_def->entity_name);
+ sqlite3IdListDelete(db, trigger_def->cols);
+ sql_expr_delete(db, trigger_def->when, false);
if (parse->parsed_ast.trigger == NULL)
sql_trigger_delete(db, trigger);
else
@@ -424,11 +424,11 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
void
sql_drop_trigger(struct Parse *parser)
{
- struct drop_entity_def drop_def = parser->drop_entity_def;
- struct alter_entity_def alter_def = drop_def.base;
- assert(alter_def.entity_type == ENTITY_TYPE_TRIGGER);
- struct SrcList *name = alter_def.entity_name;
- bool no_err = drop_def.if_exist;
+ struct drop_entity_def *drop_def = &parser->drop_entity_def;
+ struct alter_entity_def *alter_def = &drop_def->base;
+ assert(alter_def->entity_type == ENTITY_TYPE_TRIGGER);
+ struct SrcList *name = alter_def->entity_name;
+ bool no_err = drop_def->if_exist;
sqlite3 *db = parser->db;
if (db->mallocFailed)
goto drop_trigger_cleanup;
>> + assert(alter_def.entity_type == ENTITY_TYPE_FK);
>> /*
>> * When this function is called second time during
>> * <CREATE TABLE ...> statement (i.e. at VDBE runtime),
>> @@ -2211,19 +2227,29 @@ constraint_is_named(const char *name)
>> }
>> void
>> -sql_create_index(struct Parse *parse, struct Token *token,
>> - struct SrcList *tbl_name, struct ExprList *col_list,
>> - MAYBE_UNUSED struct Token *start, enum sort_order sort_order,
>> - bool if_not_exist, enum sql_index_type idx_type) {
>> +sql_create_index(struct Parse *parse) {
>> /* The index to be created. */
>> struct index *index = NULL;
>> /* Name of the index. */
>> char *name = NULL;
>> struct sqlite3 *db = parse->db;
>> assert(!db->init.busy);
>> + struct create_index_def create_idx_def = parse->create_index_def;
>> + struct create_entity_def *create_entity_def =
>> + (struct create_entity_def *) &create_idx_def;
>
> 4. Please, use 'base'. Code would be smaller and less erroneous than casts.
> Or at least be consistent and use everywhere either cast or base.
Initially I tended to use cast when we didn’t need intermediate structs,
instead of base.base.base…
For instance, in sql_add_check_constraint():
struct create_ck_def *ck_def = &parser->create_ck_def;
struct alter_entity_def *alter_def =
(struct alter_entity_def *) &parser->create_ck_def;
Becomes:
struct alter_entity_def *alter_def =
&parser->create_ck_def.base.base.base;
IMHO base looks worse than cast in this case.
I’ve fixed this in several other places, so if I am not
mistaken this case (sql_add_check_constraint()) is
the only one where I use cast; it can be fixed as well,
if you wish.
>> + struct alter_entity_def alter_entity_def = create_entity_def->base;
>> + assert(alter_entity_def.entity_type == ENTITY_TYPE_INDEX);
>> + /*
>> + * Get list of columns to be indexed. It will be NULL if
>> + * this is a primary key or unique-constraint on the most
>> + * recent column added to the table under construction.
>> + */
>> + struct ExprList *col_list = create_idx_def.cols;
>> + struct SrcList *tbl_name = alter_entity_def.entity_name;
>> if (db->mallocFailed || parse->nErr > 0)
>> goto exit_create_index;
>> + enum sql_index_type idx_type = create_idx_def.idx_type;
>> if (idx_type == SQL_INDEX_TYPE_UNIQUE ||
>> idx_type == SQL_INDEX_TYPE_NON_UNIQUE) {
>> Vdbe *v = sqlite3GetVdbe(parse);
>> @@ -2299,11 +2324,10 @@ sql_create_index(struct Parse *parse, struct Token *token,
>> }
>> } else {
>> char *constraint_name = NULL;
>> - if (parse->constraintName.z != NULL)
>> + if (create_entity_def->name.n > 0)
>
> 5. In struct Token comment you wrote/copy-pasted, that
> if z == NULL, n is undefined. So here n > 0 does
> not mean that z != NULL. Or the comment was wrong?
In this particular case name token is a part of create_entity_def
struct. In turn create_entity_def is filled with 0 or both fields (z and n)
are initialised properly. But anyway, just in case I removed that comment.
>> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
>> index 0bcf41594..e8b053361 100644
>> --- a/src/box/sql/parse.y
>> +++ b/src/box/sql/parse.y
>> @@ -168,7 +168,9 @@ cmd ::= ROLLBACK TO savepoint_opt nm(X). {
>> //
>> cmd ::= create_table create_table_args.
>> create_table ::= createkw TABLE ifnotexists(E) nm(Y). {
>> - sqlite3StartTable(pParse,&Y,E);
>> + alter_entity_def_init(&pParse->create_table_def, NULL, ENTITY_TYPE_TABLE);
>> + create_entity_def_init(&pParse->create_table_def, Y, E);
>
> 6. We never use 'init' suffix.
I grepped a lot ‘init’ suffixes around source code:
credentials_init()
mpstream_init()
tuple_init()
cmsg_init()
vy_log_record_init()
and so forth.
> Please, use 'create’.
But then it would look a bit weird:
create_entity_def_create()
Two ‘create’ words in one name, isn’t it too much?
Moreover, I believe ‘init’ is more suitable, since in fact
we initialise already created struct. Instead of ‘init’ I can
suggest ‘_prepare’, ‘_assebmle’, ‘_fill'
If you are sure about ‘_create' naming, I will fix it.
>> + sqlite3StartTable(pParse);
>> }
>> createkw(A) ::= CREATE(A). {disableLookaside(pParse);}
>> @@ -237,8 +239,15 @@ nm(A) ::= id(A). {
>> carglist ::= carglist cconsdef.
>> carglist ::= .
>> cconsdef ::= cconsname ccons.
>> -cconsname ::= CONSTRAINT nm(X). {pParse->constraintName = X;}
>> -cconsname ::= . {pParse->constraintName.n = 0;}
>> +cconsname ::= cconsname_start cconsname_parse .
>> +cconsname_start ::= . {
>> + /* Prepare base members for re-usage. */
>> + memset(&pParse->create_index_def, 0, sizeof(struct create_index_def));
>> +}
>> +cconsname_parse ::= CONSTRAINT nm(X). {
>> + create_entity_def_init(&pParse->create_index_def, X, false);
>
> 7. Why in case of any constraint start, you reset only index_def? UNIQUE/PK
> are not the only existing constraints.
> Also, memset(0) is not scalable
> solution - you need a separate reset() function, or something like that in
> case if in future parse_def.h structures will grow in size and complexity,
> and 0 becomes not default value of some attribute.
Well, imho it is too much. Actually, here we really care only about
zeroing several pointers and some size params and don’t care
about the rest. This is first rule to be called when some constraint
is created. What is more, now terminal initialisers fill all fields, except
for base structs. Hence, now it is enough to reset memory layout
up to struct create_constraint_def:
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 589b13d76..abb2c689e 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -242,7 +243,7 @@ cconsdef ::= cconsname ccons.
cconsname ::= cconsname_start cconsname_parse .
cconsname_start ::= . {
/* Prepare base members for re-usage. */
- memset(&pParse->create_index_def, 0, sizeof(struct create_index_def));
+ memset(&pParse->create_index_def, 0, sizeof(struct create_constraint_def));
}
cconsname_parse ::= CONSTRAINT nm(X). {
create_entity_def_init(&pParse->create_index_def, X, false);
> By the way, 0 is already not default value of entity type. Memset above
> makes create_index_def ha ENTITY_TYPE_TABLE type.
It is OK, proper type is set in terminal initialisers, as you suggested.
>> @@ -260,14 +269,29 @@ ccons ::= NULL onconf(R). {
>> sql_column_add_nullable_action(pParse, R);
>> }
>> ccons ::= NOT NULL onconf(R). {sql_column_add_nullable_action(pParse, R);}
>> -ccons ::= PRIMARY KEY sortorder(Z) autoinc(I).
>> - {sqlite3AddPrimaryKey(pParse,0,I,Z);}
>> -ccons ::= UNIQUE. {sql_create_index(pParse,0,0,0,0,
>> - SORT_ORDER_ASC, false,
>> - SQL_INDEX_TYPE_CONSTRAINT_UNIQUE);}
>> -ccons ::= CHECK LP expr(X) RP. {sql_add_check_constraint(pParse,&X);}
>> -ccons ::= REFERENCES nm(T) eidlist_opt(TA) refargs(R).
>> - {sql_create_foreign_key(pParse, NULL, NULL, NULL, &T, TA, false, R);}
>> +ccons ::= PRIMARY KEY sortorder(Z) autoinc(I). {
>> + pParse->create_table_def.has_autoinc = I;
>> + sqlite3AddPrimaryKey(pParse,0,Z);
>> +}
>> +ccons ::= UNIQUE. {
>> + pParse->create_index_def.idx_type = SQL_INDEX_TYPE_CONSTRAINT_UNIQUE;
>> + entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
>
> 8. Why UNIQUE sets entity type index, but PRIMARY KEY does not?
In fact, it does - inside sqlite3AddPrimaryKey function.
But now I see that it would be better to move it outside
function. Refactored a bit:
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index b6f099cf5..475ee58fa 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -668,14 +667,12 @@ field_def_create_for_pk(struct Parse *parser, struct field_def *field,
* index for the key. No index is created for INTEGER PRIMARY KEYs.
*/
void
-sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
- ExprList * pList, /* List of field names to be indexed */
- enum sort_order sortOrder
- )
+sqlite3AddPrimaryKey(struct Parse *pParse)
{
Table *pTab = pParse->create_table_def.new_table;
int iCol = -1, i;
int nTerm;
+ struct ExprList *pList = pParse->create_index_def.cols;
if (pTab == 0)
goto primary_key_exit;
if (sql_table_primary_key(pTab) != NULL) {
@@ -708,12 +705,8 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
}
}
}
- pParse->create_index_def.idx_type = SQL_INDEX_TYPE_CONSTRAINT_PK;
- pParse->create_index_def.sort_order = sortOrder;
- entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
if (nTerm == 1 && iCol != -1 &&
- pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER &&
- sortOrder != SORT_ORDER_DESC) {
+ pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER) {
struct sqlite3 *db = pParse->db;
struct ExprList *list;
struct Token token;
@@ -732,9 +725,8 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
"INTEGER PRIMARY KEY or INT PRIMARY KEY");
goto primary_key_exit;
} else {
- pParse->create_index_def.cols = pList;
sql_create_index(pParse);
- pList = 0;
+ pList = NULL;
if (pParse->nErr > 0)
goto primary_key_exit;
}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index e8b053361..cf302f496 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -271,11 +271,15 @@ ccons ::= NULL onconf(R). {
ccons ::= NOT NULL onconf(R). {sql_column_add_nullable_action(pParse, R);}
ccons ::= PRIMARY KEY sortorder(Z) autoinc(I). {
pParse->create_table_def.has_autoinc = I;
- sqlite3AddPrimaryKey(pParse,0,Z);
+ entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
+ create_index_def_init(&pParse->create_index_def, NULL,
+ SQL_INDEX_TYPE_CONSTRAINT_PK, Z);
+ sqlite3AddPrimaryKey(pParse);
}
ccons ::= UNIQUE. {
- pParse->create_index_def.idx_type = SQL_INDEX_TYPE_CONSTRAINT_UNIQUE;
entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
+ create_index_def_init(&pParse->create_index_def, NULL,
+ SQL_INDEX_TYPE_CONSTRAINT_UNIQUE, SORT_ORDER_ASC);
sql_create_index(pParse);
}
@@ -334,11 +338,14 @@ init_deferred_pred_opt(A) ::= INITIALLY IMMEDIATE. {A = 0;}
tconsdef ::= cconsname tcons.
tcons ::= PRIMARY KEY LP sortlist(X) autoinc(I) RP. {
pParse->create_table_def.has_autoinc = I;
- sqlite3AddPrimaryKey(pParse, X, 0);
+ entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
+ create_index_def_init(&pParse->create_index_def, X,
+ SQL_INDEX_TYPE_CONSTRAINT_PK, SORT_ORDER_ASC);
+ sqlite3AddPrimaryKey(pParse);
}
tcons ::= UNIQUE LP sortlist(X) RP. {
- pParse->create_index_def.cols = X;
- pParse->create_index_def.idx_type = SQL_INDEX_TYPE_CONSTRAINT_UNIQUE;
+ create_index_def_init(&pParse->create_index_def, X,
+ SQL_INDEX_TYPE_CONSTRAINT_UNIQUE, SORT_ORDER_ASC);
entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
sql_create_index(pParse);
}
@@ -1243,8 +1250,7 @@ cmd ::= createkw uniqueflag(U) INDEX ifnotexists(NE) nm(X)
sqlite3SrcListAppend(pParse->db, 0, &Y),
ENTITY_TYPE_INDEX);
create_entity_def_init(&pParse->create_index_def, X, NE);
- pParse->create_index_def.idx_type = U;
- pParse->create_index_def.cols = Z;
+ create_index_def_init(&pParse->create_index_def, Z, U, SORT_ORDER_ASC);
sql_create_index(pParse);
}
index 7a61a27e0..dbeb11892 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -245,6 +245,15 @@ drop_entity_def_init(struct drop_entity_def *drop_def, struct Token name,
drop_def->if_exist = if_exist;
}
+static inline void
+create_index_def_init(struct create_index_def *index_def, struct ExprList *cols,
+ enum sql_index_type idx_type, enum sort_order sort_order)
+{
+ index_def->cols = cols;
+ index_def->idx_type = idx_type;
+ index_def->sort_order = sort_order;
+}
+
>> diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
>> +enum entity_type {
>> + ENTITY_TYPE_TABLE = 0,
>> + ENTITY_TYPE_INDEX,
>> + ENTITY_TYPE_TRIGGER,
>> + ENTITY_TYPE_CK,
>> + ENTITY_TYPE_FK,
>> + entity_type_MAX
>
> 9. These entity_types are useless. Even being used in asserts,
> they do not prevent errors, allowing to create an instance of
> create_entity_def, but use it as drop_entity_def, because
> base.entity_type in both cases can be, for example,
> ENTITY_TYPE_INDEX.
>
> Please, use not object entity types, but def types:
>
> ALTER_DEF_CREATE_TABLE/DROP_TABLE/RENAME.../...INDEX.../...
>
> Or split entity type and action, so as to have something like
> this:
>
> def.entity = ALTER_DEF_TABLE;
> def.action = ALTER_DEF_RENAME;
I’d better choose this way. Sorry, I can’t spot diff
of particularly this fix, since it has been entangled with diffs
to other comments. But I will attach whole diff at the end of letter.
> By the way, please, rename ENTITY_TYPE_CK to ENTITY_TYPE_CHECK.
> Three letters 'HEC' economy is not worth the name obfuscation.
It is common abbreviation (the same as FK), and AFAIR was even
suggested by Konstantin in one of reviews (mb not to this patch).
It simply makes name be similar to ENTITY_TYPE_FK.
>> +/**
>> + * In those functions which accept void * instead of certain type
>> + * it was made on purpose: it allows to avoid explicit cast before
>> + * passing parameter to function. Hence, we can invoke it like:
>> + * entity_set_type(create_fk_def, ...); instead of
>> + * entity_set_type((struct alter_entity_def *) create_fk_def, ...);
>> + */
>> +static inline void
>> +entity_set_type(void *entity_def, enum entity_type type)
>> +{
>> + struct alter_entity_def *alter_def =
>> + (struct alter_entity_def *) entity_def;
>> + alter_def->entity_type = type;
>> +}
>
> 10. Please, do not use void * in either of these functions. Use
> normal types. Also, entity_set_type() should not exist as a separate
> function. Instead, each terminal structure above should have it own
> create() function, which internally sets type.
That’s fair:
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index cf302f496..15b2b478c 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -271,13 +271,11 @@ ccons ::= NULL onconf(R). {
ccons ::= NOT NULL onconf(R). {sql_column_add_nullable_action(pParse, R);}
ccons ::= PRIMARY KEY sortorder(Z) autoinc(I). {
pParse->create_table_def.has_autoinc = I;
- entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
create_index_def_init(&pParse->create_index_def, NULL,
SQL_INDEX_TYPE_CONSTRAINT_PK, Z);
sqlite3AddPrimaryKey(pParse);
}
ccons ::= UNIQUE. {
- entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
create_index_def_init(&pParse->create_index_def, NULL,
SQL_INDEX_TYPE_CONSTRAINT_UNIQUE, SORT_ORDER_ASC);
sql_create_index(pParse);
@@ -286,14 +284,12 @@ ccons ::= UNIQUE. {
ccons ::= check_constraint_def .
check_constraint_def ::= CHECK LP expr(X) RP. {
- pParse->create_ck_def.expr = &X;
- entity_set_type(&pParse->create_index_def, ENTITY_TYPE_CK);
+ create_ck_def_init(&pParse->create_ck_def, &X);
sql_add_check_constraint(pParse);
}
ccons ::= REFERENCES nm(T) eidlist_opt(TA) refargs(R). {
create_fk_def_init(&pParse->create_fk_def, NULL, &T, TA, R);
- entity_set_type(&pParse->create_fk_def, ENTITY_TYPE_FK);
sql_create_foreign_key(pParse);
}
ccons ::= defer_subclause(D). {fkey_change_defer_mode(pParse, D);}
@@ -1369,11 +1365,7 @@ trigger_decl(A) ::= TRIGGER ifnotexists(NOERR) nm(B)
ON fullname(E) foreach_clause when_clause(G). {
struct create_trigger_def *trigger_def = &pParse->create_trigger_def;
alter_entity_def_init(trigger_def, E, ENTITY_TYPE_TRIGGER);
create_entity_def_init(trigger_def, B, NOERR);
- trigger_def->tr_tm = C;
- trigger_def->op = D.a;
- trigger_def->cols = D.b;
- trigger_def->when = G;
+ create_trigger_def_init(&trigger_def, C, D.a, D.b, G);
sql_trigger_begin(pParse);
A = B; /*A-overwrites-T*/
}
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index dbeb11892..2da678d77 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -74,8 +74,6 @@
/**
* Each token coming out of the lexer is an instance of
* this structure. Tokens are also used as part of an expression.
- *
- * Note if Token.z is NULL then Token.n is undefined.
*/
struct Token {
/** Text of the token. Not NULL-terminated! */
@@ -107,9 +105,17 @@ enum entity_type {
entity_type_MAX
};
+enum alter_action {
+ ALTER_ACTION_CREATE = 0,
+ ALTER_ACTION_DROP,
+ ALTER_ACTION_RENAME
+};
+
struct alter_entity_def {
/** Type of topmost entity. */
enum entity_type entity_type;
+ /** Action to be performed using current entity. */
+ enum alter_action alter_action;
/** As a rule it is a name of table to be altered. */
struct SrcList *entity_name;
};
@@ -202,13 +208,6 @@ struct create_index_def {
* entity_set_type(create_fk_def, ...); instead of
* entity_set_type((struct alter_entity_def *) create_fk_def, ...);
*/
-static inline void
-entity_set_type(void *entity_def, enum entity_type type)
-{
- struct alter_entity_def *alter_def =
- (struct alter_entity_def *) entity_def;
- alter_def->entity_type = type;
-}
static inline void
entity_set_defer_mode(void *entity_def, bool is_deferred)
@@ -245,6 +244,25 @@ drop_entity_def_init(struct drop_entity_def *drop_def, struct Token name,
drop_def->if_exist = if_exist;
}
+static inline void
+create_trigger_def_init(struct create_trigger_def *trigger_def, int tr_tm,
+ int op, struct IdList *cols, struct Expr *when)
+{
+ trigger_def->tr_tm = tr_tm;
+ trigger_def->op = op;
+ trigger_def->cols = cols;
+ trigger_def->when = when;
+ ((struct alter_entity_def *) trigger_def_def)->entity_type =
+ ENTITY_TYPE_TRIGGER;
+}
+
+static inline void
+create_ck_def_init(struct create_ck_def *ck_def, struct ExprSpan *expr)
+{
+ ck_def->expr = expr;
+ ((struct alter_entity_def *) ck_def)->entity_type = ENTITY_TYPE_CK;
+}
+
static inline void
create_index_def_init(struct create_index_def *index_def, struct ExprList *cols,
enum sql_index_type idx_type, enum sort_order sort_order)
@@ -252,6 +270,7 @@ create_index_def_init(struct create_index_def *index_def, struct ExprList *cols,
index_def->cols = cols;
index_def->idx_type = idx_type;
index_def->sort_order = sort_order;
+ ((struct alter_entity_def *) index_def)->entity_type = ENTITY_TYPE_INDEX;
}
static inline void
@@ -264,6 +283,7 @@ create_fk_def_init(struct create_fk_def *fk_def, struct ExprList *child_cols,
fk_def->parent_name = parent_name;
fk_def->parent_cols = parent_cols;
fk_def->actions = actions;
+ ((struct alter_entity_def *) fk_def)->entity_type = ENTITY_TYPE_FK;
}
I know you asked me to choose between base struct and cast.
However, I think here cast is much more suitable: these structures
are terminated so we have to write base.base.base.
On the other hand, almost in all places one or two nesting
base usage looks better. I can revert this change with ease,
if you want to.
Eliminating void * diff (note that I still use cast to jump over
several base structs at once; again, if you feel that I should
use base.base.base - I will fix it):
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index dbeb11892..0392c0a87 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -195,54 +229,71 @@ struct create_index_def {
enum sort_order sort_order;
};
-/**
- * In those functions which accept void * instead of certain type
- * it was made on purpose: it allows to avoid explicit cast before
- * passing parameter to function. Hence, we can invoke it like:
- * entity_set_type(create_fk_def, ...); instead of
- * entity_set_type((struct alter_entity_def *) create_fk_def, ...);
- */
+/** Basic initialisers of parse structures.*/
static inline void
-entity_set_type(void *entity_def, enum entity_type type)
+alter_entity_def_init(struct alter_entity_def *alter_def,
+ struct SrcList *entity_name)
{
- struct alter_entity_def *alter_def =
- (struct alter_entity_def *) entity_def;
- alter_def->entity_type = type;
+ alter_def->entity_name = entity_name;
}
static inline void
-entity_set_defer_mode(void *entity_def, bool is_deferred)
+rename_entity_def_init(struct rename_entity_def *rename_def,
+ struct Token new_name)
{
- struct create_constraint_def *constr_def =
- (struct create_constraint_def *) entity_def;
- constr_def->is_deferred = is_deferred;
+ rename_def->new_name = new_name;
+ struct alter_entity_def *alter_def =
+ (struct alter_entity_def *) rename_def;
+ alter_def->entity_type = ENTITY_TYPE_TABLE;
+ alter_def->alter_action = ALTER_ACTION_RENAME;
}
static inline void
-alter_entity_def_init(void *entity_def, struct SrcList *entity_name,
- enum entity_type type)
+create_entity_def_init(struct create_entity_def *create_def, struct Token name,
+ bool if_not_exist)
{
- struct alter_entity_def *alter_def =
- (struct alter_entity_def *) entity_def;
- alter_def->entity_name = entity_name;
- alter_def->entity_type = type;
+ create_def->name = name;
+ create_def->if_not_exist = if_not_exist;
}
static inline void
-create_entity_def_init(void *entity_def, struct Token name, bool if_not_exist)
+create_constraint_def_init(struct create_constraint_def *constr_def,
+ bool is_deferred)
{
- struct create_entity_def *create_def =
- (struct create_entity_def *) entity_def;
- create_def->name = name;
- create_def->if_not_exist = if_not_exist;
+ constr_def->is_deferred = is_deferred;
}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 589b13d76..22b5669d8 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -168,8 +168,9 @@ cmd ::= ROLLBACK TO savepoint_opt nm(X). {
//
cmd ::= create_table create_table_args.
create_table ::= createkw TABLE ifnotexists(E) nm(Y). {
- alter_entity_def_init(&pParse->create_table_def, NULL, ENTITY_TYPE_TABLE);
- create_entity_def_init(&pParse->create_table_def, Y, E);
+ alter_entity_def_init(&pParse->create_table_def.base.base, NULL);
+ create_entity_def_init(&pParse->create_table_def.base, Y, E);
+ create_table_def_init(&pParse->create_table_def);
sqlite3StartTable(pParse);
}
createkw(A) ::= CREATE(A). {disableLookaside(pParse);}
@@ -180,6 +181,7 @@ ifnotexists(A) ::= IF NOT EXISTS. {A = 1;}
create_table_args ::= LP columnlist RP(E). {
sqlite3EndTable(pParse,&E,0);
+ create_table_def_destroy(&pParse->create_table_def);
}
create_table_args ::= AS select(S). {
sqlite3EndTable(pParse,0,S);
@@ -242,10 +244,10 @@ cconsdef ::= cconsname ccons.
cconsname ::= cconsname_start cconsname_parse .
cconsname_start ::= . {
/* Prepare base members for re-usage. */
- memset(&pParse->create_index_def, 0, sizeof(struct create_index_def));
+ memset(&pParse->create_index_def, 0, sizeof(struct create_constraint_def));
}
cconsname_parse ::= CONSTRAINT nm(X). {
- create_entity_def_init(&pParse->create_index_def, X, false);
+ create_entity_def_init(&pParse->create_index_def.base.base, X, false);
}
cconsname_parse ::= .
ccons ::= DEFAULT term(X). {sqlite3AddDefaultValue(pParse,&X);}
@@ -346,8 +348,8 @@ tcons ::= UNIQUE LP sortlist(X) RP. {
tcons ::= check_constraint_def .
tcons ::= FOREIGN KEY LP eidlist(FA) RP
REFERENCES nm(T) eidlist_opt(TA) refargs(R) defer_subclause_opt(D). {
+ create_constraint_def_init(&pParse->create_fk_def.base, D);
create_fk_def_init(&pParse->create_fk_def, FA, &T, TA, R);
- entity_set_defer_mode(&pParse->create_fk_def, D);
sql_create_foreign_key(pParse);
}
%type defer_subclause_opt {int}
@@ -377,8 +379,9 @@ cmd ::= drop_start(X) drop_table . {
}
drop_table ::= ifexists(E) fullname(X) . {
- alter_entity_def_init(&pParse->drop_entity_def, X, ENTITY_TYPE_TABLE);
- pParse->drop_entity_def.if_exist = E;
+ alter_entity_def_init(&pParse->drop_entity_def.base, X);
+ drop_entity_def_init(&pParse->drop_entity_def, (struct Token) {}, E,
+ ENTITY_TYPE_TABLE);
}
%type drop_start {bool}
@@ -394,8 +397,7 @@ ifexists(A) ::= . {A = 0;}
cmd ::= createkw(X) VIEW ifnotexists(E) nm(Y) eidlist_opt(C)
AS select(S). {
if (!pParse->parse_only) {
- alter_entity_def_init(&pParse->create_table_def, NULL, ENTITY_TYPE_TABLE);
- create_entity_def_init(&pParse->create_table_def, Y, E);
+ create_entity_def_init(&pParse->create_table_def.base, Y, E);
sql_create_view(pParse, &X, C, S);
} else {
sql_store_select(pParse, S);
@@ -1239,10 +1241,10 @@ paren_exprlist(A) ::= LP exprlist(X) RP. {A = X;}
//
cmd ::= createkw uniqueflag(U) INDEX ifnotexists(NE) nm(X)
ON nm(Y) LP sortlist(Z) RP. {
- alter_entity_def_init(&pParse->create_index_def,
- sqlite3SrcListAppend(pParse->db, 0, &Y),
- ENTITY_TYPE_INDEX);
- create_entity_def_init(&pParse->create_index_def, X, NE);
+ alter_entity_def_init((struct alter_entity_def *) &pParse->create_index_def,
+ sqlite3SrcListAppend(pParse->db, 0, &Y));
+ create_entity_def_init((struct create_entity_def *) &pParse->create_index_def,
+ X, NE);
create_index_def_init(&pParse->create_index_def, Z, U, SORT_ORDER_ASC);
sql_create_index(pParse);
}
@@ -1307,8 +1309,8 @@ collate(C) ::= COLLATE id. {C = 1;}
///////////////////////////// The DROP INDEX command /////////////////////////
//
cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). {
- alter_entity_def_init(&pParse->drop_entity_def, Y, ENTITY_TYPE_INDEX);
- drop_entity_def_init(&pParse->drop_entity_def, X, E);
+ alter_entity_def_init(&pParse->drop_entity_def.base, Y);
+ drop_entity_def_init(&pParse->drop_entity_def, X, E, ENTITY_TYPE_INDEX);
sql_drop_index(pParse);
}
@@ -1361,8 +1363,8 @@ trigger_decl(A) ::= TRIGGER ifnotexists(NOERR) nm(B)
trigger_time(C) trigger_event(D)
ON fullname(E) foreach_clause when_clause(G). {
struct create_trigger_def *trigger_def = &pParse->create_trigger_def;
- alter_entity_def_init(trigger_def, E, ENTITY_TYPE_TRIGGER);
- create_entity_def_init(trigger_def, B, NOERR);
+ alter_entity_def_init((struct alter_entity_def *) trigger_def, E);
+ create_entity_def_init((struct create_entity_def *) trigger_def, B, NOERR);
create_trigger_def_init(trigger_def, C, D.a, D.b, G);
sql_trigger_begin(pParse);
A = B; /*A-overwrites-T*/
@@ -1474,8 +1476,9 @@ raisetype(A) ::= FAIL. {A = ON_CONFLICT_ACTION_FAIL;}
//////////////////////// DROP TRIGGER statement //////////////////////////////
cmd ::= DROP TRIGGER ifexists(NOERR) fullname(X). {
- alter_entity_def_init(&pParse->drop_entity_def, X, ENTITY_TYPE_TRIGGER);
- drop_entity_def_init(&pParse->drop_entity_def, (struct Token){}, NOERR);
+ alter_entity_def_init(&pParse->drop_entity_def.base, X);
+ drop_entity_def_init(&pParse->drop_entity_def, (struct Token){}, NOERR,
+ ENTITY_TYPE_TRIGGER);
sql_drop_trigger(pParse);
}
@@ -1485,24 +1488,24 @@ cmd ::= ANALYZE nm(X). {sqlite3Analyze(pParse, &X);}
//////////////////////// ALTER TABLE table ... ////////////////////////////////
cmd ::= ALTER TABLE fullname(X) RENAME TO nm(Z). {
- alter_entity_def_init(&pParse->rename_entity_def, X, ENTITY_TYPE_TABLE);
- pParse->rename_entity_def.new_name = Z;
+ alter_entity_def_init(&pParse->rename_entity_def.base, X);
+ rename_entity_def_init(&pParse->rename_entity_def, Z);
sql_alter_table_rename(pParse);
}
cmd ::= ALTER TABLE fullname(X) ADD CONSTRAINT nm(Z) FOREIGN KEY
LP eidlist(FA) RP REFERENCES nm(T) eidlist_opt(TA) refargs(R)
defer_subclause_opt(D). {
- alter_entity_def_init(&pParse->create_fk_def, X, ENTITY_TYPE_FK);
- create_entity_def_init(&pParse->create_fk_def, Z, false);
- entity_set_defer_mode(&pParse->create_fk_def, D);
+ alter_entity_def_init((struct alter_entity_def *) &pParse->create_fk_def, X);
+ create_entity_def_init(&pParse->create_fk_def.base.base, Z, false);
+ create_constraint_def_init(&pParse->create_fk_def.base, D);
create_fk_def_init(&pParse->create_fk_def, FA, &T, TA, R);
sql_create_foreign_key(pParse);
}
cmd ::= ALTER TABLE fullname(X) DROP CONSTRAINT nm(Z). {
- alter_entity_def_init(&pParse->drop_entity_def, X, ENTITY_TYPE_FK);
- drop_entity_def_init(&pParse->drop_entity_def, Z, false);
+ alter_entity_def_init(&pParse->drop_entity_def.base, X);
+ drop_entity_def_init(&pParse->drop_entity_def, Z, false, ENTITY_TYPE_FK);
sql_drop_foreign_key(pParse);
}
>> +static inline void
>> +entity_set_defer_mode(void *entity_def, bool is_deferred)
>> +{
>> + struct create_constraint_def *constr_def =
>> + (struct create_constraint_def *) entity_def;
>> + constr_def->is_deferred = is_deferred;
>> +}
>
> 11. Same. You unsafely cast void * to create_constraint_def *.
> Also, you have a special setter for is_deferred, but have no
> for if_exist. And nonetheless you set if_exist manually in at
> least one place. Why?
if_exist is an attribute of terminal struct (drop_entity_def), so
it can be fetched without cast/references to base struct.
On the other hand, to reach create_constraint_def we always
need to go through base structs.
In all places except this one, if_exist is initialised in drop_entity_def_init.
In this one it also can be used, but I decided to simply initialise only if_exist.
Here we don’t need set name of entity to be dropped (it is already saved in
alter_entity_def). But ok, it is not a problem to use init function here as well:
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 589b13d76..21112a49a 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -378,7 +378,7 @@ cmd ::= drop_start(X) drop_table . {
drop_table ::= ifexists(E) fullname(X) . {
alter_entity_def_init(&pParse->drop_entity_def, X, ENTITY_TYPE_TABLE);
- pParse->drop_entity_def.if_exist = E;
+ drop_entity_def_init(&pParse->drop_entity_def, (struct Token) {}, E);
}
>> +
>> +static inline void
>> +alter_entity_def_init(void *entity_def, struct SrcList *entity_name,
>> + enum entity_type type)
>
> 12. This function should not be called by user. It should be part of
> terminal def create() functions. It looks really weird when you do
> something like this:
>
> alter_entity_def_init(&pParse->create_fk_def)
> create_entity_def_init(&pParse->create_fk_def)
> entity_set_defer_mode(&pParse->create_fk_def)
>
> So basically you called three!!! constructors. It is not scalable,
> sorry.
But not always we can initialise all properties.
For instance:
ALTER TABLE t1 ADD CONSTRAINT c1 FK;
Firstly, we parse ALTER TABLE t1 and initialise
struct alter_entity_def. Then we parse ADD CONSTRAINT c1,
so initialise struct create_entity_def and struct create_constraint_def.
Finally, we get to FK properties (like list of child cols etc) and initialise
them. OK, we can set type of entity (and action) in terminal initialisation,
but we can’t set name of table, name of constraint etc. Actually, I thought
that all these refactoring is aimed at such step-by-step initialisation.
If you could suggest better approach, I would appreciate it.
>> +{
>> + struct alter_entity_def *alter_def =
>> + (struct alter_entity_def *) entity_def;
>> + alter_def->entity_name = entity_name;
>> + alter_def->entity_type = type;
>> +}
>> +
>> +static inline void
>> +create_entity_def_init(void *entity_def, struct Token name, bool if_not_exist)
>
> 13. You if_not_exist passed through the constructor of create_entity, but
> you do not do the same for is_exist and drop_entity. Why? I see the function
> below, but parse.y:381 directly changes if_exist.
Nevermind, it was only one such usage.
I fixed it, see answer to comment 11.
>> diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
>> index 824578e45..3343c2942 100644
>> --- a/src/box/sql/prepare.c
>> +++ b/src/box/sql/prepare.c
>> @@ -273,7 +273,7 @@ sql_parser_create(struct Parse *parser, sqlite3 *db)
>> {
>> memset(parser, 0, sizeof(struct Parse));
>> parser->db = db;
>> - rlist_create(&parser->new_fkey);
>> + rlist_create(&parser->create_table_def.new_fkey);
>
> 14. Please, introduce create_table_def_create() in parse_def.h and do
> all necessary actions there.
Ok (nevertheless it is the only action to be used in preparation),
see diff below.
>> rlist_create(&parser->record_list);
>> region_create(&parser->region, &cord()->slabc);
>> }
>> @@ -287,7 +287,7 @@ sql_parser_destroy(Parse *parser)
>> sqlite3DbFree(db, parser->aLabel);
>> sql_expr_list_delete(db, parser->pConstExpr);
>> struct fkey_parse *fk;
>> - rlist_foreach_entry(fk, &parser->new_fkey, link)
>> + rlist_foreach_entry(fk, &parser->create_table_def.new_fkey, link)
>> sql_expr_list_delete(db, fk->selfref_cols);
>
> 15. create_table_def_destroy().
Ok, but then we have to a) include sql.h to use sql_expr_list_delete()
b) move struct fkey_parse to parse_def as well (mb it is even better place).
Diff:
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 475ee58fa..5ddbad6e3 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1291,15 +1291,19 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
return;
int reg_space_id = getNewSpaceId(pParse);
createSpace(pParse, reg_space_id);
- /* Indexes aren't required for VIEW's.. */
- if (!p->def->opts.is_view) {
- for (uint32_t i = 0; i < p->space->index_count; ++i) {
- struct index *idx = p->space->index[i];
- vdbe_emit_create_index(pParse, p->def, idx->def,
- reg_space_id, idx->def->iid);
- }
- }
+ /*
+ * VIEWs can't have any functional parts such as
+ * indexes or foreign keys, so we can terminate
+ * right here.
+ */
+ if (p->def->opts.is_view)
+ return;
+ for (uint32_t i = 0; i < p->space->index_count; ++i) {
+ struct index *idx = p->space->index[i];
+ vdbe_emit_create_index(pParse, p->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.
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 589b13d76..3b5e03712 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -168,8 +168,9 @@ cmd ::= ROLLBACK TO savepoint_opt nm(X). {
//
cmd ::= create_table create_table_args.
create_table ::= createkw TABLE ifnotexists(E) nm(Y). {
alter_entity_def_init(&pParse->create_table_def, NULL);
create_entity_def_init(&pParse->create_table_def, Y, E);
+ create_table_def_init(&pParse->create_table_def);
sqlite3StartTable(pParse);
}
createkw(A) ::= CREATE(A). {disableLookaside(pParse);}
@@ -180,6 +181,7 @@ ifnotexists(A) ::= IF NOT EXISTS. {A = 1;}
create_table_args ::= LP columnlist RP(E). {
sqlite3EndTable(pParse,&E,0);
+ create_table_def_destroy(&pParse->create_table_def);
}
create_table_args ::= AS select(S). {
sqlite3EndTable(pParse,0,S);
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index dbeb11892..d16be2b5d 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -31,6 +31,7 @@
* SUCH DAMAGE.
*/
#include "box/fkey.h"
+#include "box/sql.h"
/**
* This file contains auxiliary structures and functions which
@@ -85,6 +84,33 @@ struct Token {
bool isReserved;
};
+/**
+ * Structure representing foreign keys constraints appeared
+ * within CREATE TABLE statement. Used only during parsing.
+ */
+struct fkey_parse {
+ /**
+ * Foreign keys constraint declared in <CREATE TABLE ...>
+ * statement. They must be coded after space creation.
+ */
+ struct fkey_def *fkey;
+ /**
+ * If inside CREATE TABLE statement we want to declare
+ * self-referenced FK constraint, we must delay their
+ * resolution until the end of parsing of all columns.
+ * E.g.: CREATE TABLE t1(id REFERENCES t1(b), b);
+ */
+ struct ExprList *selfref_cols;
+ /**
+ * Still, self-referenced columns might be NULL, if
+ * we declare FK constraints referencing PK:
+ * CREATE TABLE t1(id REFERENCES t1) - it is a valid case.
+ */
+ bool is_self_referenced;
+ /** Organize these structs into linked list. */
+ struct rlist link;
+};
+
@@ -264,6 +329,28 @@ create_fk_def_init(struct create_fk_def *fk_def, struct ExprList *child_cols,
+static inline void
+create_table_def_init(struct create_table_def *table_def)
+{
+ rlist_create(&table_def->new_fkey);
+ struct alter_entity_def *alter_def =
+ (struct alter_entity_def *) &table_def;
+ alter_def->entity_type = ENTITY_TYPE_TABLE;
+ alter_def->alter_action = ALTER_ACTION_CREATE;
+}
+
+static inline void
+create_table_def_destroy(struct create_table_def *table_def)
+{
+ struct fkey_parse *fk;
+ rlist_foreach_entry(fk, &table_def->new_fkey, link)
+ sql_expr_list_delete(sql_get(), fk->selfref_cols);
}
#endif /* TARANTOOL_BOX_SQL_PARSE_DEF_H_INCLUDED */
>> if (db != NULL) {
>> assert(db->lookaside.bDisable >=
>> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
>> index 6462467bc..e66f0d6db 100644
>> --- a/src/box/sql/resolve.c
>> +++ b/src/box/sql/resolve.c
>> @@ -1616,9 +1616,9 @@ void
>> sql_resolve_self_reference(struct Parse *parser, struct Table *table, int type,
>> struct Expr *expr, struct ExprList *expr_list)
>> {
>> - /* Fake SrcList for parser->pNewTable */
>> + /* Fake SrcList for parser->new_table */
>> SrcList sSrc;
>> - /* Name context for parser->pNewTable */
>> + /* Name context for parser->new_table */
>
> 16. No such member in struct Parse.
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index e66f0d6db..50d787d11 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -1616,9 +1616,9 @@ void
sql_resolve_self_reference(struct Parse *parser, struct Table *table, int type,
struct Expr *expr, struct ExprList *expr_list)
{
- /* Fake SrcList for parser->new_table */
+ /* Fake SrcList for parser->create_table_def */
SrcList sSrc;
- /* Name context for parser->new_table */
+ /* Name context for parser->create_table_def */
NameContext sNC;
assert(type == NC_IsCheck || type == NC_IdxExpr);
Whole diff:
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index e88b57b59..6eb14fdb5 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -40,12 +40,13 @@
void
sql_alter_table_rename(struct Parse *parse)
{
- struct rename_entity_def rename_def = parse->rename_entity_def;
- struct SrcList *src_tab = rename_def.base.entity_name;
- assert(rename_def.base.entity_type == ENTITY_TYPE_TABLE);
+ struct rename_entity_def *rename_def = &parse->rename_entity_def;
+ struct SrcList *src_tab = rename_def->base.entity_name;
+ assert(rename_def->base.entity_type == ENTITY_TYPE_TABLE);
+ assert(rename_def->base.alter_action == ALTER_ACTION_RENAME);
assert(src_tab->nSrc == 1);
struct sqlite3 *db = parse->db;
- char *new_name = sqlite3NameFromToken(db, &rename_def.new_name);
+ char *new_name = sqlite3NameFromToken(db, &rename_def->new_name);
if (new_name == NULL)
goto exit_rename_table;
/* Check that new name isn't occupied by another table. */
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index b6f099cf5..5ddbad6e3 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -383,7 +383,7 @@ sql_table_new(Parse *parser, char *name)
* when the "TEMP" or "TEMPORARY" keyword occurs in between
* CREATE and TABLE.
*
- * The new table record is initialized and put in pParse->new_table.
+ * The new table record is initialized and put in pParse->create_table_def.
* As more of the CREATE TABLE statement is parsed, additional action
* routines will be called to add more information to this record.
* At the end of the CREATE TABLE statement, the sqlite3EndTable() routine
@@ -401,8 +401,7 @@ sqlite3StartTable(struct Parse *pParse)
if (v == NULL)
goto cleanup;
sqlite3VdbeCountChanges(v);
- struct Token name =
- ((struct create_entity_def *) &pParse->create_table_def)->name;
+ struct Token name = pParse->create_table_def.base.name;
zName = sqlite3NameFromToken(db, &name);
if (zName == 0)
@@ -668,14 +667,12 @@ field_def_create_for_pk(struct Parse *parser, struct field_def *field,
* index for the key. No index is created for INTEGER PRIMARY KEYs.
*/
void
-sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
- ExprList * pList, /* List of field names to be indexed */
- enum sort_order sortOrder
- )
+sqlite3AddPrimaryKey(struct Parse *pParse)
{
Table *pTab = pParse->create_table_def.new_table;
int iCol = -1, i;
int nTerm;
+ struct ExprList *pList = pParse->create_index_def.cols;
if (pTab == 0)
goto primary_key_exit;
if (sql_table_primary_key(pTab) != NULL) {
@@ -708,12 +705,8 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
}
}
}
- pParse->create_index_def.idx_type = SQL_INDEX_TYPE_CONSTRAINT_PK;
- pParse->create_index_def.sort_order = sortOrder;
- entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
if (nTerm == 1 && iCol != -1 &&
- pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER &&
- sortOrder != SORT_ORDER_DESC) {
+ pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER) {
struct sqlite3 *db = pParse->db;
struct ExprList *list;
struct Token token;
@@ -732,9 +725,8 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
"INTEGER PRIMARY KEY or INT PRIMARY KEY");
goto primary_key_exit;
} else {
- pParse->create_index_def.cols = pList;
sql_create_index(pParse);
- pList = 0;
+ pList = NULL;
if (pParse->nErr > 0)
goto primary_key_exit;
}
@@ -777,8 +769,7 @@ sql_add_check_constraint(struct Parse *parser)
sqlite3DbFree(parser->db, expr->u.zToken);
goto release_expr;
}
- struct create_entity_def *entity_def =
- (struct create_entity_def *) ck_def;
+ struct create_entity_def *entity_def = &ck_def->base.base;
if (entity_def->name.n > 0) {
sqlite3ExprListSetName(parser, table->def->opts.checks,
&entity_def->name, 1);
@@ -1300,15 +1291,19 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
return;
int reg_space_id = getNewSpaceId(pParse);
createSpace(pParse, reg_space_id);
- /* Indexes aren't required for VIEW's.. */
- if (!p->def->opts.is_view) {
- for (uint32_t i = 0; i < p->space->index_count; ++i) {
- struct index *idx = p->space->index[i];
- vdbe_emit_create_index(pParse, p->def, idx->def,
- reg_space_id, idx->def->iid);
- }
- }
+ /*
+ * VIEWs can't have any functional parts such as
+ * indexes or foreign keys, so we can terminate
+ * right here.
+ */
+ if (p->def->opts.is_view)
+ return;
+ for (uint32_t i = 0; i < p->space->index_count; ++i) {
+ struct index *idx = p->space->index[i];
+ vdbe_emit_create_index(pParse, p->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.
@@ -1697,6 +1692,7 @@ sql_drop_table(struct Parse *parse_context, bool is_view)
{
struct drop_entity_def drop_def = parse_context->drop_entity_def;
assert(drop_def.base.entity_type == ENTITY_TYPE_TABLE);
+ assert(drop_def.base.alter_action == ALTER_ACTION_DROP);
struct SrcList *table_name_list = drop_def.base.entity_name;
struct Vdbe *v = sqlite3GetVdbe(parse_context);
struct sqlite3 *db = parse_context->db;
@@ -1798,11 +1794,12 @@ void
sql_create_foreign_key(struct Parse *parse_context)
{
struct sqlite3 *db = parse_context->db;
- struct create_fk_def create_fk_def = parse_context->create_fk_def;
- struct create_constraint_def create_constr_def = create_fk_def.base;
- struct create_entity_def create_def = create_constr_def.base;
- struct alter_entity_def alter_def = create_def.base;
- assert(alter_def.entity_type == ENTITY_TYPE_FK);
+ struct create_fk_def *create_fk_def = &parse_context->create_fk_def;
+ struct create_constraint_def *create_constr_def = &create_fk_def->base;
+ struct create_entity_def *create_def = &create_constr_def->base;
+ struct alter_entity_def *alter_def = &create_def->base;
+ assert(alter_def->entity_type == ENTITY_TYPE_FK);
+ assert(alter_def->alter_action == ALTER_ACTION_CREATE);
/*
* When this function is called second time during
* <CREATE TABLE ...> statement (i.e. at VDBE runtime),
@@ -1826,7 +1823,7 @@ sql_create_foreign_key(struct Parse *parse_context)
/* Whether we are processing ALTER TABLE or CREATE TABLE. */
bool is_alter = new_tab == NULL;
uint32_t child_cols_count;
- struct ExprList *child_cols = create_fk_def.child_cols;
+ struct ExprList *child_cols = create_fk_def->child_cols;
if (child_cols == NULL) {
assert(!is_alter);
child_cols_count = 1;
@@ -1835,7 +1832,7 @@ sql_create_foreign_key(struct Parse *parse_context)
}
struct space *child_space = NULL;
if (is_alter) {
- const char *child_name = alter_def.entity_name->a[0].zName;
+ const char *child_name = alter_def->entity_name->a[0].zName;
child_space = space_by_name(child_name);
if (child_space == NULL) {
diag_set(ClientError, ER_NO_SUCH_SPACE, child_name);
@@ -1852,7 +1849,7 @@ sql_create_foreign_key(struct Parse *parse_context)
memset(fk, 0, sizeof(*fk));
rlist_add_entry(&table_def->new_fkey, fk, link);
}
- struct Token *parent = create_fk_def.parent_name;
+ struct Token *parent = create_fk_def->parent_name;
assert(parent != NULL);
parent_name = sqlite3NameFromToken(db, parent);
if (parent_name == NULL)
@@ -1864,7 +1861,7 @@ sql_create_foreign_key(struct Parse *parse_context)
*/
is_self_referenced = !is_alter &&
strcmp(parent_name, new_tab->def->name) == 0;
- struct ExprList *parent_cols = create_fk_def.parent_cols;
+ struct ExprList *parent_cols = create_fk_def->parent_cols;
struct space *parent_space = space_by_name(parent_name);
if (parent_space == NULL) {
if (is_self_referenced) {
@@ -1885,18 +1882,18 @@ sql_create_foreign_key(struct Parse *parse_context)
}
}
if (!is_alter) {
- if (create_def.name.n == 0) {
+ if (create_def->name.n == 0) {
constraint_name =
sqlite3MPrintf(db, "FK_CONSTRAINT_%d_%s",
++table_def->fkey_count,
new_tab->def->name);
} else {
constraint_name =
- sqlite3NameFromToken(db, &create_def.name);
+ sqlite3NameFromToken(db, &create_def->name);
}
} else {
constraint_name =
- sqlite3NameFromToken(db, &create_def.name);
+ sqlite3NameFromToken(db, &create_def->name);
}
if (constraint_name == NULL)
goto exit_create_fk;
@@ -1929,11 +1926,11 @@ sql_create_foreign_key(struct Parse *parse_context)
diag_set(OutOfMemory, fk_size, "region", "struct fkey");
goto tnt_error;
}
- int actions = create_fk_def.actions;
+ int actions = create_fk_def->actions;
fk->field_count = child_cols_count;
fk->child_id = child_space != NULL ? child_space->def->id : 0;
fk->parent_id = parent_space != NULL ? parent_space->def->id : 0;
- fk->is_deferred = create_constr_def.is_deferred;
+ fk->is_deferred = create_constr_def->is_deferred;
fk->match = (enum fkey_match) ((actions >> 16) & 0xff);
fk->on_update = (enum fkey_action) ((actions >> 8) & 0xff);
fk->on_delete = (enum fkey_action) (actions & 0xff);
@@ -2021,9 +2018,10 @@ fkey_change_defer_mode(struct Parse *parse_context, bool is_deferred)
void
sql_drop_foreign_key(struct Parse *parse_context)
{
- struct drop_entity_def drop_def = parse_context->drop_entity_def;
- assert(drop_def.base.entity_type == ENTITY_TYPE_FK);
- const char *table_name = drop_def.base.entity_name->a[0].zName;
+ struct drop_entity_def *drop_def = &parse_context->drop_entity_def;
+ assert(drop_def->base.entity_type == ENTITY_TYPE_FK);
+ assert(drop_def->base.alter_action == ALTER_ACTION_DROP);
+ const char *table_name = drop_def->base.entity_name->a[0].zName;
assert(table_name != NULL);
struct space *child = space_by_name(table_name);
if (child == NULL) {
@@ -2033,7 +2031,7 @@ sql_drop_foreign_key(struct Parse *parse_context)
return;
}
char *constraint_name = sqlite3NameFromToken(parse_context->db,
- &drop_def.name);
+ &drop_def->name);
if (constraint_name != NULL)
vdbe_emit_fkey_drop(parse_context, constraint_name,
child->def->id);
@@ -2234,22 +2232,22 @@ sql_create_index(struct Parse *parse) {
char *name = NULL;
struct sqlite3 *db = parse->db;
assert(!db->init.busy);
- struct create_index_def create_idx_def = parse->create_index_def;
- struct create_entity_def *create_entity_def =
- (struct create_entity_def *) &create_idx_def;
- struct alter_entity_def alter_entity_def = create_entity_def->base;
- assert(alter_entity_def.entity_type == ENTITY_TYPE_INDEX);
+ struct create_index_def *create_idx_def = &parse->create_index_def;
+ struct create_entity_def *create_entity_def = &create_idx_def->base.base;
+ struct alter_entity_def *alter_entity_def = &create_entity_def->base;
+ assert(alter_entity_def->entity_type == ENTITY_TYPE_INDEX);
+ assert(alter_entity_def->alter_action == ALTER_ACTION_CREATE);
/*
* Get list of columns to be indexed. It will be NULL if
* this is a primary key or unique-constraint on the most
* recent column added to the table under construction.
*/
- struct ExprList *col_list = create_idx_def.cols;
- struct SrcList *tbl_name = alter_entity_def.entity_name;
+ struct ExprList *col_list = create_idx_def->cols;
+ struct SrcList *tbl_name = alter_entity_def->entity_name;
if (db->mallocFailed || parse->nErr > 0)
goto exit_create_index;
- enum sql_index_type idx_type = create_idx_def.idx_type;
+ enum sql_index_type idx_type = create_idx_def->idx_type;
if (idx_type == SQL_INDEX_TYPE_UNIQUE ||
idx_type == SQL_INDEX_TYPE_NON_UNIQUE) {
Vdbe *v = sqlite3GetVdbe(parse);
@@ -2386,7 +2384,7 @@ sql_create_index(struct Parse *parse) {
goto exit_create_index;
assert(col_list->nExpr == 1);
sqlite3ExprListSetSortOrder(col_list,
- create_idx_def.sort_order);
+ create_idx_def->sort_order);
} else {
sqlite3ExprListCheckLength(parse, col_list, "index");
}
@@ -2559,14 +2557,15 @@ sql_create_index(struct Parse *parse) {
void
sql_drop_index(struct Parse *parse_context)
{
- struct drop_entity_def drop_def = parse_context->drop_entity_def;
- assert(drop_def.base.entity_type == ENTITY_TYPE_INDEX);
+ struct drop_entity_def *drop_def = &parse_context->drop_entity_def;
+ assert(drop_def->base.entity_type == ENTITY_TYPE_INDEX);
+ assert(drop_def->base.alter_action == ALTER_ACTION_DROP);
struct Vdbe *v = sqlite3GetVdbe(parse_context);
assert(v != NULL);
struct sqlite3 *db = parse_context->db;
/* Never called with prior errors. */
assert(parse_context->nErr == 0);
- struct SrcList *table_list = drop_def.base.entity_name;
+ struct SrcList *table_list = drop_def->base.entity_name;
assert(table_list->nSrc == 1);
char *table_name = table_list->a[0].zName;
const char *index_name = NULL;
@@ -2575,14 +2574,14 @@ sql_drop_index(struct Parse *parse_context)
}
sqlite3VdbeCountChanges(v);
struct space *space = space_by_name(table_name);
- bool if_exists = drop_def.if_exist;
+ bool if_exists = drop_def->if_exist;
if (space == NULL) {
if (!if_exists)
sqlite3ErrorMsg(parse_context, "no such space: %s",
table_name);
goto exit_drop_index;
}
- index_name = sqlite3NameFromToken(db, &drop_def.name);
+ index_name = sqlite3NameFromToken(db, &drop_def->name);
uint32_t index_id = box_index_id_by_name(space->def->id, index_name,
strlen(index_name));
if (index_id == BOX_ID_NIL) {
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index e8b053361..22b5669d8 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -168,8 +168,9 @@ cmd ::= ROLLBACK TO savepoint_opt nm(X). {
//
cmd ::= create_table create_table_args.
create_table ::= createkw TABLE ifnotexists(E) nm(Y). {
- alter_entity_def_init(&pParse->create_table_def, NULL, ENTITY_TYPE_TABLE);
- create_entity_def_init(&pParse->create_table_def, Y, E);
+ alter_entity_def_init(&pParse->create_table_def.base.base, NULL);
+ create_entity_def_init(&pParse->create_table_def.base, Y, E);
+ create_table_def_init(&pParse->create_table_def);
sqlite3StartTable(pParse);
}
createkw(A) ::= CREATE(A). {disableLookaside(pParse);}
@@ -180,6 +181,7 @@ ifnotexists(A) ::= IF NOT EXISTS. {A = 1;}
create_table_args ::= LP columnlist RP(E). {
sqlite3EndTable(pParse,&E,0);
+ create_table_def_destroy(&pParse->create_table_def);
}
create_table_args ::= AS select(S). {
sqlite3EndTable(pParse,0,S);
@@ -242,10 +244,10 @@ cconsdef ::= cconsname ccons.
cconsname ::= cconsname_start cconsname_parse .
cconsname_start ::= . {
/* Prepare base members for re-usage. */
- memset(&pParse->create_index_def, 0, sizeof(struct create_index_def));
+ memset(&pParse->create_index_def, 0, sizeof(struct create_constraint_def));
}
cconsname_parse ::= CONSTRAINT nm(X). {
- create_entity_def_init(&pParse->create_index_def, X, false);
+ create_entity_def_init(&pParse->create_index_def.base.base, X, false);
}
cconsname_parse ::= .
ccons ::= DEFAULT term(X). {sqlite3AddDefaultValue(pParse,&X);}
@@ -271,25 +273,25 @@ ccons ::= NULL onconf(R). {
ccons ::= NOT NULL onconf(R). {sql_column_add_nullable_action(pParse, R);}
ccons ::= PRIMARY KEY sortorder(Z) autoinc(I). {
pParse->create_table_def.has_autoinc = I;
- sqlite3AddPrimaryKey(pParse,0,Z);
+ create_index_def_init(&pParse->create_index_def, NULL,
+ SQL_INDEX_TYPE_CONSTRAINT_PK, Z);
+ sqlite3AddPrimaryKey(pParse);
}
ccons ::= UNIQUE. {
- pParse->create_index_def.idx_type = SQL_INDEX_TYPE_CONSTRAINT_UNIQUE;
- entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
+ create_index_def_init(&pParse->create_index_def, NULL,
+ SQL_INDEX_TYPE_CONSTRAINT_UNIQUE, SORT_ORDER_ASC);
sql_create_index(pParse);
}
ccons ::= check_constraint_def .
check_constraint_def ::= CHECK LP expr(X) RP. {
- pParse->create_ck_def.expr = &X;
- entity_set_type(&pParse->create_index_def, ENTITY_TYPE_CK);
+ create_ck_def_init(&pParse->create_ck_def, &X);
sql_add_check_constraint(pParse);
}
ccons ::= REFERENCES nm(T) eidlist_opt(TA) refargs(R). {
create_fk_def_init(&pParse->create_fk_def, NULL, &T, TA, R);
- entity_set_type(&pParse->create_fk_def, ENTITY_TYPE_FK);
sql_create_foreign_key(pParse);
}
ccons ::= defer_subclause(D). {fkey_change_defer_mode(pParse, D);}
@@ -334,20 +336,20 @@ init_deferred_pred_opt(A) ::= INITIALLY IMMEDIATE. {A = 0;}
tconsdef ::= cconsname tcons.
tcons ::= PRIMARY KEY LP sortlist(X) autoinc(I) RP. {
pParse->create_table_def.has_autoinc = I;
- sqlite3AddPrimaryKey(pParse, X, 0);
+ create_index_def_init(&pParse->create_index_def, X,
+ SQL_INDEX_TYPE_CONSTRAINT_PK, SORT_ORDER_ASC);
+ sqlite3AddPrimaryKey(pParse);
}
tcons ::= UNIQUE LP sortlist(X) RP. {
- pParse->create_index_def.cols = X;
- pParse->create_index_def.idx_type = SQL_INDEX_TYPE_CONSTRAINT_UNIQUE;
- entity_set_type(&pParse->create_index_def, ENTITY_TYPE_INDEX);
+ create_index_def_init(&pParse->create_index_def, X,
+ SQL_INDEX_TYPE_CONSTRAINT_UNIQUE, SORT_ORDER_ASC);
sql_create_index(pParse);
}
tcons ::= check_constraint_def .
tcons ::= FOREIGN KEY LP eidlist(FA) RP
REFERENCES nm(T) eidlist_opt(TA) refargs(R) defer_subclause_opt(D). {
+ create_constraint_def_init(&pParse->create_fk_def.base, D);
create_fk_def_init(&pParse->create_fk_def, FA, &T, TA, R);
- entity_set_defer_mode(&pParse->create_fk_def, D);
- entity_set_type(&pParse->create_fk_def, ENTITY_TYPE_FK);
sql_create_foreign_key(pParse);
}
%type defer_subclause_opt {int}
@@ -377,8 +379,9 @@ cmd ::= drop_start(X) drop_table . {
}
drop_table ::= ifexists(E) fullname(X) . {
- alter_entity_def_init(&pParse->drop_entity_def, X, ENTITY_TYPE_TABLE);
- pParse->drop_entity_def.if_exist = E;
+ alter_entity_def_init(&pParse->drop_entity_def.base, X);
+ drop_entity_def_init(&pParse->drop_entity_def, (struct Token) {}, E,
+ ENTITY_TYPE_TABLE);
}
%type drop_start {bool}
@@ -394,8 +397,7 @@ ifexists(A) ::= . {A = 0;}
cmd ::= createkw(X) VIEW ifnotexists(E) nm(Y) eidlist_opt(C)
AS select(S). {
if (!pParse->parse_only) {
- alter_entity_def_init(&pParse->create_table_def, NULL, ENTITY_TYPE_TABLE);
- create_entity_def_init(&pParse->create_table_def, Y, E);
+ create_entity_def_init(&pParse->create_table_def.base, Y, E);
sql_create_view(pParse, &X, C, S);
} else {
sql_store_select(pParse, S);
@@ -1239,12 +1241,11 @@ paren_exprlist(A) ::= LP exprlist(X) RP. {A = X;}
//
cmd ::= createkw uniqueflag(U) INDEX ifnotexists(NE) nm(X)
ON nm(Y) LP sortlist(Z) RP. {
- alter_entity_def_init(&pParse->create_index_def,
- sqlite3SrcListAppend(pParse->db, 0, &Y),
- ENTITY_TYPE_INDEX);
- create_entity_def_init(&pParse->create_index_def, X, NE);
- pParse->create_index_def.idx_type = U;
- pParse->create_index_def.cols = Z;
+ alter_entity_def_init((struct alter_entity_def *) &pParse->create_index_def,
+ sqlite3SrcListAppend(pParse->db, 0, &Y));
+ create_entity_def_init((struct create_entity_def *) &pParse->create_index_def,
+ X, NE);
+ create_index_def_init(&pParse->create_index_def, Z, U, SORT_ORDER_ASC);
sql_create_index(pParse);
}
@@ -1308,8 +1309,8 @@ collate(C) ::= COLLATE id. {C = 1;}
///////////////////////////// The DROP INDEX command /////////////////////////
//
cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). {
- alter_entity_def_init(&pParse->drop_entity_def, Y, ENTITY_TYPE_INDEX);
- drop_entity_def_init(&pParse->drop_entity_def, X, E);
+ alter_entity_def_init(&pParse->drop_entity_def.base, Y);
+ drop_entity_def_init(&pParse->drop_entity_def, X, E, ENTITY_TYPE_INDEX);
sql_drop_index(pParse);
}
@@ -1362,12 +1363,9 @@ trigger_decl(A) ::= TRIGGER ifnotexists(NOERR) nm(B)
trigger_time(C) trigger_event(D)
ON fullname(E) foreach_clause when_clause(G). {
struct create_trigger_def *trigger_def = &pParse->create_trigger_def;
- alter_entity_def_init(trigger_def, E, ENTITY_TYPE_TRIGGER);
- create_entity_def_init(trigger_def, B, NOERR);
- trigger_def->tr_tm = C;
- trigger_def->op = D.a;
- trigger_def->cols = D.b;
- trigger_def->when = G;
+ alter_entity_def_init((struct alter_entity_def *) trigger_def, E);
+ create_entity_def_init((struct create_entity_def *) trigger_def, B, NOERR);
+ create_trigger_def_init(trigger_def, C, D.a, D.b, G);
sql_trigger_begin(pParse);
A = B; /*A-overwrites-T*/
}
@@ -1478,8 +1476,9 @@ raisetype(A) ::= FAIL. {A = ON_CONFLICT_ACTION_FAIL;}
//////////////////////// DROP TRIGGER statement //////////////////////////////
cmd ::= DROP TRIGGER ifexists(NOERR) fullname(X). {
- alter_entity_def_init(&pParse->drop_entity_def, X, ENTITY_TYPE_TRIGGER);
- drop_entity_def_init(&pParse->drop_entity_def, (struct Token){}, NOERR);
+ alter_entity_def_init(&pParse->drop_entity_def.base, X);
+ drop_entity_def_init(&pParse->drop_entity_def, (struct Token){}, NOERR,
+ ENTITY_TYPE_TRIGGER);
sql_drop_trigger(pParse);
}
@@ -1489,24 +1488,24 @@ cmd ::= ANALYZE nm(X). {sqlite3Analyze(pParse, &X);}
//////////////////////// ALTER TABLE table ... ////////////////////////////////
cmd ::= ALTER TABLE fullname(X) RENAME TO nm(Z). {
- alter_entity_def_init(&pParse->rename_entity_def, X, ENTITY_TYPE_TABLE);
- pParse->rename_entity_def.new_name = Z;
+ alter_entity_def_init(&pParse->rename_entity_def.base, X);
+ rename_entity_def_init(&pParse->rename_entity_def, Z);
sql_alter_table_rename(pParse);
}
cmd ::= ALTER TABLE fullname(X) ADD CONSTRAINT nm(Z) FOREIGN KEY
LP eidlist(FA) RP REFERENCES nm(T) eidlist_opt(TA) refargs(R)
defer_subclause_opt(D). {
- alter_entity_def_init(&pParse->create_fk_def, X, ENTITY_TYPE_FK);
- create_entity_def_init(&pParse->create_fk_def, Z, false);
- entity_set_defer_mode(&pParse->create_fk_def, D);
+ alter_entity_def_init((struct alter_entity_def *) &pParse->create_fk_def, X);
+ create_entity_def_init(&pParse->create_fk_def.base.base, Z, false);
+ create_constraint_def_init(&pParse->create_fk_def.base, D);
create_fk_def_init(&pParse->create_fk_def, FA, &T, TA, R);
sql_create_foreign_key(pParse);
}
cmd ::= ALTER TABLE fullname(X) DROP CONSTRAINT nm(Z). {
- alter_entity_def_init(&pParse->drop_entity_def, X, ENTITY_TYPE_FK);
- drop_entity_def_init(&pParse->drop_entity_def, Z, false);
+ alter_entity_def_init(&pParse->drop_entity_def.base, X);
+ drop_entity_def_init(&pParse->drop_entity_def, Z, false, ENTITY_TYPE_FK);
sql_drop_foreign_key(pParse);
}
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index 7a61a27e0..0392c0a87 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -31,6 +31,7 @@
* SUCH DAMAGE.
*/
#include "box/fkey.h"
+#include "box/sql.h"
/**
* This file contains auxiliary structures and functions which
@@ -74,8 +75,6 @@
/**
* Each token coming out of the lexer is an instance of
* this structure. Tokens are also used as part of an expression.
- *
- * Note if Token.z is NULL then Token.n is undefined.
*/
struct Token {
/** Text of the token. Not NULL-terminated! */
@@ -85,6 +84,33 @@ struct Token {
bool isReserved;
};
+/**
+ * Structure representing foreign keys constraints appeared
+ * within CREATE TABLE statement. Used only during parsing.
+ */
+struct fkey_parse {
+ /**
+ * Foreign keys constraint declared in <CREATE TABLE ...>
+ * statement. They must be coded after space creation.
+ */
+ struct fkey_def *fkey;
+ /**
+ * If inside CREATE TABLE statement we want to declare
+ * self-referenced FK constraint, we must delay their
+ * resolution until the end of parsing of all columns.
+ * E.g.: CREATE TABLE t1(id REFERENCES t1(b), b);
+ */
+ struct ExprList *selfref_cols;
+ /**
+ * Still, self-referenced columns might be NULL, if
+ * we declare FK constraints referencing PK:
+ * CREATE TABLE t1(id REFERENCES t1) - it is a valid case.
+ */
+ bool is_self_referenced;
+ /** Organize these structs into linked list. */
+ struct rlist link;
+};
+
/**
* Possible SQL index types. Note that PK and UNIQUE constraints
* are implemented as indexes and have their own types:
@@ -107,9 +133,17 @@ enum entity_type {
entity_type_MAX
};
+enum alter_action {
+ ALTER_ACTION_CREATE = 0,
+ ALTER_ACTION_DROP,
+ ALTER_ACTION_RENAME
+};
+
struct alter_entity_def {
/** Type of topmost entity. */
enum entity_type entity_type;
+ /** Action to be performed using current entity. */
+ enum alter_action alter_action;
/** As a rule it is a name of table to be altered. */
struct SrcList *entity_name;
};
@@ -195,54 +229,84 @@ struct create_index_def {
enum sort_order sort_order;
};
-/**
- * In those functions which accept void * instead of certain type
- * it was made on purpose: it allows to avoid explicit cast before
- * passing parameter to function. Hence, we can invoke it like:
- * entity_set_type(create_fk_def, ...); instead of
- * entity_set_type((struct alter_entity_def *) create_fk_def, ...);
- */
+/** Basic initialisers of parse structures.*/
static inline void
-entity_set_type(void *entity_def, enum entity_type type)
+alter_entity_def_init(struct alter_entity_def *alter_def,
+ struct SrcList *entity_name)
{
- struct alter_entity_def *alter_def =
- (struct alter_entity_def *) entity_def;
- alter_def->entity_type = type;
+ alter_def->entity_name = entity_name;
}
static inline void
-entity_set_defer_mode(void *entity_def, bool is_deferred)
+rename_entity_def_init(struct rename_entity_def *rename_def,
+ struct Token new_name)
{
- struct create_constraint_def *constr_def =
- (struct create_constraint_def *) entity_def;
- constr_def->is_deferred = is_deferred;
+ rename_def->new_name = new_name;
+ struct alter_entity_def *alter_def =
+ (struct alter_entity_def *) rename_def;
+ alter_def->entity_type = ENTITY_TYPE_TABLE;
+ alter_def->alter_action = ALTER_ACTION_RENAME;
}
static inline void
-alter_entity_def_init(void *entity_def, struct SrcList *entity_name,
- enum entity_type type)
+create_entity_def_init(struct create_entity_def *create_def, struct Token name,
+ bool if_not_exist)
{
- struct alter_entity_def *alter_def =
- (struct alter_entity_def *) entity_def;
- alter_def->entity_name = entity_name;
- alter_def->entity_type = type;
+ create_def->name = name;
+ create_def->if_not_exist = if_not_exist;
}
static inline void
-create_entity_def_init(void *entity_def, struct Token name, bool if_not_exist)
+create_constraint_def_init(struct create_constraint_def *constr_def,
+ bool is_deferred)
{
- struct create_entity_def *create_def =
- (struct create_entity_def *) entity_def;
- create_def->name = name;
- create_def->if_not_exist = if_not_exist;
+ constr_def->is_deferred = is_deferred;
}
static inline void
drop_entity_def_init(struct drop_entity_def *drop_def, struct Token name,
- bool if_exist)
+ bool if_exist, enum entity_type entity_type)
{
drop_def->name = name;
drop_def->if_exist = if_exist;
+ drop_def->base.entity_type = entity_type;
+ drop_def->base.alter_action = ALTER_ACTION_DROP;
+}
+
+static inline void
+create_trigger_def_init(struct create_trigger_def *trigger_def, int tr_tm,
+ int op, struct IdList *cols, struct Expr *when) {
+ trigger_def->tr_tm = tr_tm;
+ trigger_def->op = op;
+ trigger_def->cols = cols;
+ trigger_def->when = when;
+ struct alter_entity_def *alter_def =
+ (struct alter_entity_def *) trigger_def;
+ alter_def->entity_type = ENTITY_TYPE_TRIGGER;
+ alter_def->alter_action = ALTER_ACTION_CREATE;
+}
+
+static inline void
+create_ck_def_init(struct create_ck_def *ck_def, struct ExprSpan *expr)
+{
+ ck_def->expr = expr;
+ struct alter_entity_def *alter_def =
+ (struct alter_entity_def *) ck_def;
+ alter_def->entity_type = ENTITY_TYPE_CK;
+ alter_def->alter_action = ALTER_ACTION_CREATE;
+}
+
+static inline void
+create_index_def_init(struct create_index_def *index_def, struct ExprList *cols,
+ enum sql_index_type idx_type, enum sort_order sort_order)
+{
+ index_def->cols = cols;
+ index_def->idx_type = idx_type;
+ index_def->sort_order = sort_order;
+ struct alter_entity_def *alter_def =
+ (struct alter_entity_def *) index_def;
+ alter_def->entity_type = ENTITY_TYPE_INDEX;
+ alter_def->alter_action = ALTER_ACTION_CREATE;
}
static inline void
@@ -255,6 +319,28 @@ create_fk_def_init(struct create_fk_def *fk_def, struct ExprList *child_cols,
fk_def->parent_name = parent_name;
fk_def->parent_cols = parent_cols;
fk_def->actions = actions;
+ struct alter_entity_def *alter_def =
+ (struct alter_entity_def *) fk_def;
+ alter_def->entity_type = ENTITY_TYPE_FK;
+ alter_def->alter_action = ALTER_ACTION_CREATE;
+}
+
+static inline void
+create_table_def_init(struct create_table_def *table_def)
+{
+ rlist_create(&table_def->new_fkey);
+ struct alter_entity_def *alter_def =
+ (struct alter_entity_def *) table_def;
+ alter_def->entity_type = ENTITY_TYPE_TABLE;
+ alter_def->alter_action = ALTER_ACTION_CREATE;
+}
+
+static inline void
+create_table_def_destroy(struct create_table_def *table_def)
+{
+ struct fkey_parse *fk;
+ rlist_foreach_entry(fk, &table_def->new_fkey, link)
+ sql_expr_list_delete(sql_get(), fk->selfref_cols);
}
#endif /* TARANTOOL_BOX_SQL_PARSE_DEF_H_INCLUDED */
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 3343c2942..c2660eeff 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -273,7 +273,6 @@ sql_parser_create(struct Parse *parser, sqlite3 *db)
{
memset(parser, 0, sizeof(struct Parse));
parser->db = db;
- rlist_create(&parser->create_table_def.new_fkey);
rlist_create(&parser->record_list);
region_create(&parser->region, &cord()->slabc);
}
@@ -286,9 +285,6 @@ sql_parser_destroy(Parse *parser)
sqlite3 *db = parser->db;
sqlite3DbFree(db, parser->aLabel);
sql_expr_list_delete(db, parser->pConstExpr);
- struct fkey_parse *fk;
- rlist_foreach_entry(fk, &parser->create_table_def.new_fkey, link)
- sql_expr_list_delete(db, fk->selfref_cols);
if (db != NULL) {
assert(db->lookaside.bDisable >=
parser->disableLookaside);
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index e66f0d6db..50d787d11 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -1616,9 +1616,9 @@ void
sql_resolve_self_reference(struct Parse *parser, struct Table *table, int type,
struct Expr *expr, struct ExprList *expr_list)
{
- /* Fake SrcList for parser->new_table */
+ /* Fake SrcList for parser->create_table_def */
SrcList sSrc;
- /* Name context for parser->new_table */
+ /* Name context for parser->create_table_def */
NameContext sNC;
assert(type == NC_IsCheck || type == NC_IdxExpr);
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 4ee2ed5cf..4e6806be8 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2638,33 +2638,6 @@ enum ast_type {
ast_type_MAX
};
-/**
- * Structure representing foreign keys constraints appeared
- * within CREATE TABLE statement. Used only during parsing.
- */
-struct fkey_parse {
- /**
- * Foreign keys constraint declared in <CREATE TABLE ...>
- * statement. They must be coded after space creation.
- */
- struct fkey_def *fkey;
- /**
- * If inside CREATE TABLE statement we want to declare
- * self-referenced FK constraint, we must delay their
- * resolution until the end of parsing of all columns.
- * E.g.: CREATE TABLE t1(id REFERENCES t1(b), b);
- */
- struct ExprList *selfref_cols;
- /**
- * Still, self-referenced columns might be NULL, if
- * we declare FK constraints referencing PK:
- * CREATE TABLE t1(id REFERENCES t1) - it is a valid case.
- */
- bool is_self_referenced;
- /** Organize these structs into linked list. */
- struct rlist link;
-};
-
/*
* An SQL parser context. A copy of this structure is passed through
* the parser and down into all the parser action routine in order to
@@ -3350,7 +3323,8 @@ void
sql_column_add_nullable_action(struct Parse *parser,
enum on_conflict_action nullable_action);
-void sqlite3AddPrimaryKey(Parse *, ExprList *, enum sort_order);
+void
+sqlite3AddPrimaryKey(struct Parse *parse);
/**
* Add a new CHECK constraint to the table currently under
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 33b5ea7c4..3456a244d 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -68,24 +68,25 @@ sql_trigger_begin(struct Parse *parse)
struct sql_trigger *trigger = NULL;
/* The database connection. */
struct sqlite3 *db = parse->db;
- struct create_trigger_def trigger_def = parse->create_trigger_def;
- struct create_entity_def create_def = trigger_def.base;
- struct alter_entity_def alter_def = create_def.base;
- assert(alter_def.entity_type == ENTITY_TYPE_TRIGGER);
+ struct create_trigger_def *trigger_def = &parse->create_trigger_def;
+ struct create_entity_def *create_def = &trigger_def->base;
+ struct alter_entity_def *alter_def = &create_def->base;
+ assert(alter_def->entity_type == ENTITY_TYPE_TRIGGER);
+ assert(alter_def->alter_action == ALTER_ACTION_CREATE);
char *trigger_name = NULL;
- if (alter_def.entity_name == NULL || db->mallocFailed)
+ if (alter_def->entity_name == NULL || db->mallocFailed)
goto trigger_cleanup;
- assert(alter_def.entity_name->nSrc == 1);
- assert(create_def.name.n > 0);
- trigger_name = sqlite3NameFromToken(db, &create_def.name);
+ assert(alter_def->entity_name->nSrc == 1);
+ assert(create_def->name.n > 0);
+ trigger_name = sqlite3NameFromToken(db, &create_def->name);
if (trigger_name == NULL)
goto trigger_cleanup;
if (sqlite3CheckIdentifierName(parse, trigger_name) != SQLITE_OK)
goto trigger_cleanup;
- const char *table_name = alter_def.entity_name->a[0].zName;
+ const char *table_name = alter_def->entity_name->a[0].zName;
uint32_t space_id;
if (schema_find_id(BOX_SPACE_ID, 2, table_name, strlen(table_name),
&space_id) != 0)
@@ -108,7 +109,7 @@ sql_trigger_begin(struct Parse *parse)
int name_reg = ++parse->nMem;
sqlite3VdbeAddOp4(parse->pVdbe, OP_String8, 0, name_reg, 0,
name_copy, P4_DYNAMIC);
- bool no_err = create_def.if_not_exist;
+ bool no_err = create_def->if_not_exist;
if (vdbe_emit_halt_with_presence_test(parse, BOX_TRIGGER_ID, 0,
name_reg, 1,
ER_TRIGGER_EXISTS,
@@ -126,12 +127,12 @@ sql_trigger_begin(struct Parse *parse)
trigger->space_id = space_id;
trigger->zName = trigger_name;
trigger_name = NULL;
- assert(trigger_def.op == TK_INSERT || trigger_def.op == TK_UPDATE ||
- trigger_def.op== TK_DELETE);
- trigger->op = (u8) trigger_def.op;
- trigger->tr_tm = trigger_def.tr_tm;
- trigger->pWhen = sqlite3ExprDup(db, trigger_def.when, EXPRDUP_REDUCE);
- trigger->pColumns = sqlite3IdListDup(db, trigger_def.cols);
+ assert(trigger_def->op == TK_INSERT || trigger_def->op == TK_UPDATE ||
+ trigger_def->op== TK_DELETE);
+ trigger->op = (u8) trigger_def->op;
+ trigger->tr_tm = trigger_def->tr_tm;
+ trigger->pWhen = sqlite3ExprDup(db, trigger_def->when, EXPRDUP_REDUCE);
+ trigger->pColumns = sqlite3IdListDup(db, trigger_def->cols);
if ((trigger->pWhen != NULL && trigger->pWhen == NULL) ||
(trigger->pColumns != NULL && trigger->pColumns == NULL))
goto trigger_cleanup;
@@ -141,9 +142,9 @@ sql_trigger_begin(struct Parse *parse)
trigger_cleanup:
sqlite3DbFree(db, trigger_name);
- sqlite3SrcListDelete(db, alter_def.entity_name);
- sqlite3IdListDelete(db, trigger_def.cols);
- sql_expr_delete(db, trigger_def.when, false);
+ sqlite3SrcListDelete(db, alter_def->entity_name);
+ sqlite3IdListDelete(db, trigger_def->cols);
+ sql_expr_delete(db, trigger_def->when, false);
if (parse->parsed_ast.trigger == NULL)
sql_trigger_delete(db, trigger);
else
@@ -424,11 +425,12 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
void
sql_drop_trigger(struct Parse *parser)
{
- struct drop_entity_def drop_def = parser->drop_entity_def;
- struct alter_entity_def alter_def = drop_def.base;
- assert(alter_def.entity_type == ENTITY_TYPE_TRIGGER);
- struct SrcList *name = alter_def.entity_name;
- bool no_err = drop_def.if_exist;
+ struct drop_entity_def *drop_def = &parser->drop_entity_def;
+ struct alter_entity_def *alter_def = &drop_def->base;
+ assert(alter_def->entity_type == ENTITY_TYPE_TRIGGER);
+ assert(alter_def->alter_action == ALTER_ACTION_DROP);
+ struct SrcList *name = alter_def->entity_name;
+ bool no_err = drop_def->if_exist;
sqlite3 *db = parser->db;
if (db->mallocFailed)
goto drop_trigger_cleanup;
next prev parent reply other threads:[~2019-02-08 14:25 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-23 17:56 [tarantool-patches] [PATCH v2 0/5] Introduce ALTER TABLE ADD CONSTRAINT UNIQUE/PK Nikita Pettik
2019-01-23 17:56 ` [tarantool-patches] [PATCH v2 1/5] sql: introduce structs assembling DDL arguments during parsing Nikita Pettik
2019-01-24 8:36 ` [tarantool-patches] " Konstantin Osipov
2019-01-24 10:47 ` n.pettik
2019-01-24 12:30 ` Konstantin Osipov
2019-01-29 19:03 ` n.pettik
2019-01-29 19:29 ` Vladislav Shpilevoy
2019-01-29 20:04 ` n.pettik
2019-01-29 20:20 ` Vladislav Shpilevoy
2019-01-29 21:25 ` n.pettik
2019-01-31 19:32 ` n.pettik
2019-02-04 15:25 ` Vladislav Shpilevoy
2019-02-08 14:25 ` n.pettik [this message]
2019-02-15 20:13 ` Vladislav Shpilevoy
2019-02-27 22:56 ` n.pettik
2019-03-12 12:50 ` Vladislav Shpilevoy
2019-03-14 18:13 ` n.pettik
2019-03-25 11:25 ` Vladislav Shpilevoy
2019-03-26 18:01 ` n.pettik
2019-03-26 18:06 ` Vladislav Shpilevoy
2019-03-27 13:00 ` n.pettik
2019-03-27 13:29 ` Vladislav Shpilevoy
2019-03-27 13:44 ` n.pettik
2019-03-27 14:03 ` Vladislav Shpilevoy
2019-03-27 14:11 ` n.pettik
2019-01-23 17:56 ` [tarantool-patches] [PATCH v2 2/5] sql: rework ALTER TABLE grammar Nikita Pettik
2019-01-23 17:56 ` [tarantool-patches] [PATCH v2 3/5] sql: refactor getNewIid() function Nikita Pettik
2019-01-23 17:56 ` [tarantool-patches] [PATCH v2 4/5] sql: fix error message for improperly created index Nikita Pettik
2019-02-08 17:14 ` [tarantool-patches] " Konstantin Osipov
2019-01-23 17:56 ` [tarantool-patches] [PATCH v2 5/5] sql: introduce ALTER TABLE ADD CONSTRAINT UNIQUE/PRIMARY KEY Nikita Pettik
2019-01-24 8:31 ` [tarantool-patches] " Konstantin Osipov
2019-01-29 19:29 ` Vladislav Shpilevoy
2019-02-08 17:16 ` Konstantin Osipov
2019-02-08 17:36 ` n.pettik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CC4E5BAB-1117-43AB-AC4A-3BC91E706294@tarantool.org \
--to=korablev@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=v.shpilevoy@tarantool.org \
--subject='[tarantool-patches] Re: [PATCH v2 1/5] sql: introduce structs assembling DDL arguments during parsing' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox