Thanks for the fixes! See my answers, 8 comments and a
commit on the branch.
On 25/07/2018 13:03, n.pettik wrote:
Also see other fixes on the branch in a separate commit.
Thx for fixes. I have squashed them all.
Except fixes mentioned below, I disabled (temporary) sql-tap/alter2.test.lua
(it checks work of ALTER TABLE ADD CONSTRAINT) for vinyl engine.
Since in previous patch we prohibited creation of FK constraints on
non-empty spaces and as condition used ‘index_size()’, some tests turn out
to be flaky. (I don’t think that we should disable these tests for vinyl, but
didn’t
come up with satisfactory solution.)
+ !fkey_child_is_modified(fk_def, aChange))
continue;
- }
-
@@ -977,100 +686,74 @@ sqlite3FkCheck(Parse * pParse, /* Parse context */
* might be set incorrectly if any OP_FkCounter
related scans are
* omitted.
*/
- if (!pFKey->isDeferred && eAction != OE_Cascade
- && eAction != OE_SetNull) {
+ if (!fk_def->is_deferred &&
+ action != FKEY_ACTION_CASCADE &&
+ action != FKEY_ACTION_SET_NULL) {
sqlite3MayAbort(pParse);
}
}
- pItem->zName = 0;
sqlite3SrcListDelete(db, pSrc);
}
- sqlite3DbFree(db, aiCol);
}
}
#define COLUMN_MASK(x) (((x)>31) ? 0xffffffff : ((u32)1<<(x)))
24. Lets use 64 bitmask and utilities from column_mask.h.
It is not so easy to do: the same mask is used for triggers as well.
So if we want to change format of mast for FK, we should do it
almost everywhere in SQL source code. I may do it as separate
follow-up patch, if you wish.
-/*
- * This function is called before generating code to update or delete a
- * row contained in table pTab.
- */
-u32
-sqlite3FkOldmask(Parse * pParse, /* Parse context */
- Table * pTab /* Table being modified */
- )
+uint32_t
+fkey_old_mask(uint32_t space_id)
25. I think we should calculate this mask once on fk creation
like it is done for key_def.columnm_mask.
In fact, this mask is calculated for whole space (i.e. all of its FK
constraints),
not for particular FK. So basically, we need to add this mask to space_def/space
and update on each FK creation. Is this OK?
diff --git a/extra/mkopcodeh.sh b/extra/mkopcodeh.sh
index 63ad0d56a..9e97a50f0 100755
--- a/extra/mkopcodeh.sh
+++ b/extra/mkopcodeh.sh
@@ -220,8 +224,12 @@ while [ "$i" -lt "$nOp" ]; do
i=$((i + 1))
done
max="$cnt"
+echo "//*************** $max $nOp $mxTk"
@@ -283,11 +303,16 @@ printf '%s\n' "#define OPFLG_OUT2 0x10 /* out2: P2 is
an output */"
printf '%s\n' "#define OPFLG_OUT3 0x20 /* out3: P3 is an output */"
printf '%s\n' "#define OPFLG_INITIALIZER {\\"
i=0
-while [ "$i" -le "$max" ]; do
+while [ "$i" -le "$mxTk" ]; do
if [ "$((i % 8))" -eq 0 ]; then
printf '/* %3d */' "$i"
fi
- eval "bv=\$ARRAY_bv_$i"
+ eval "is_exists=\${ARRAY_bv_$i:-}"
+ if [ ! -n "$is_exists" ] ; then
+ bv=0
+ else
+ eval "bv=\$ARRAY_bv_$i"
+ fi
printf ' 0x%02x,' "$bv"
if [ "$((i % 8))" -eq 7 ]; then
printf '%s\n' "\\"
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index 8c1c36b9b..0e770272e 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -190,12 +189,6 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token *
pColDef)
sqlite3ErrorMsg(pParse, "Cannot add a UNIQUE column");
return;
}
- if ((user_session->sql_flags & SQLITE_ForeignKeys) && pNew->pFKey
- && pDflt) {
- sqlite3ErrorMsg(pParse,
- "Cannot add a REFERENCES column with non-NULL
default value");
- return;
- }
assert(pNew->def->fields[pNew->def->field_count - 1].is_nullable ==
action_is_nullable(pNew->def->fields[
pNew->def->field_count - 1].nullable_action));
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 789a628d6..fc097a319 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
+
+static int
+resolve_link(struct Parse *parse_context, const struct space_def *def,
+ const char *field_name, uint32_t *link)
+{
+ assert(link != NULL);
+ for (uint32_t j = 0; j < def->field_count; ++j) {
+ if (strcmp(field_name, def->fields[j].name) == 0) {
+ *link = j;
+ return 0;
+ }
+ }
+ sqlite3ErrorMsg(parse_context, "unknown column %s in foreign key "
+ "definition", field_name);
+ return -1;
+}
+
/*
* This routine is called to report the final ")" that terminates
* a CREATE TABLE statement.
@@ -1720,6 +1803,43 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
/* Reparse everything to update our internal data structures */
parseTableSchemaRecord(pParse, iSpaceId, zStmt); /*
consumes zStmt */
+
+ /* Code creation of FK constraints, if any. */
+ struct fkey_parse *fk_parse;
+ rlist_foreach_entry(fk_parse, &pParse->new_fkey, link) {
+ struct fkey_def *fk = fk_parse->fkey;
+ if (fk_parse->selfref_cols != NULL) {
+ struct ExprList *cols = fk_parse->selfref_cols;
+ for (uint32_t i = 0; i < fk->field_count; ++i) {
+ if (resolve_link(pParse, p->def,
+ cols->a[i].zName,
+
&fk->links[i].parent_field) != 0)
+ return;
+ }
+ fk->parent_id = iSpaceId;
+ } else if (fk_parse->is_self_referenced) {
+ struct Index *pk = sqlite3PrimaryKeyIndex(p);
+ if (pk->def->key_def->part_count !=
+ fk->field_count) {
+ diag_set(ClientError,
+ ER_CREATE_FK_CONSTRAINT,
+ fk->name, "number of columns "
+ "in foreign key does not "
+ "match the number of columns "
+ "in the referenced table");
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
+ return;
+ }
+ for (uint32_t i = 0; i < fk->field_count; ++i) {
+ fk->links[i].parent_field =
+
pk->def->key_def->parts[i].fieldno;
+ }
+ fk->parent_id = iSpaceId;
+ }
+ fk->child_id = iSpaceId;
+ vdbe_emit_fkey_create(pParse, fk);
+ }
}
/* Add the table to the in-memory representation of the database.
@@ -2106,177 +2262,280 @@ sql_drop_table(struct Parse *parse_context, struct
SrcList *table_name_list,
sqlite3SrcListDelete(db, table_name_list);
}
-/*
- * This routine is called to create a new foreign key on the table
- * currently under construction. pFromCol determines which columns
- * in the current table point to the foreign key. If pFromCol==0 then
- * connect the key to the last column inserted. pTo is the name of
- * the table referred to (a.k.a the "parent" table). pToCol is a list
- * of tables in the parent pTo table. flags contains all
- * information about the conflict resolution algorithms specified
- * in the ON DELETE, ON UPDATE and ON INSERT clauses.
+/**
+ * Return ordinal number of column by name. In case of error,
+ * set error message.
*
- * An FKey structure is created and added to the table currently
- * under construction in the pParse->pNewTable field.
+ * @param parse_context Parsing context.
+ * @param space Space which column belongs to.
+ * @param column_name Name of column to investigate.
+ * @param[out] colno Found name of column.
*
- * The foreign key is set for IMMEDIATE processing. A subsequent call
- * to sqlite3DeferForeignKey() might change this to DEFERRED.
+ * @retval 0 on success, -1 on fault.
*/
+static int
+columnno_by_name(struct Parse *parse_context, const struct space *space,
+ const char *column_name, uint32_t *colno)
+{
+ assert(colno != NULL);
+ uint32_t column_len = strlen(column_name);
+ if (tuple_fieldno_by_name(space->def->dict, column_name, column_len,
+ field_name_hash(column_name, column_len),
+ colno) != 0) {
+ sqlite3ErrorMsg(parse_context,
+ "table \"%s\" doesn't feature column %s",
+ space->def->name, column_name);
+ return -1;
+ }
+ return 0;
+}
+
void
-sqlite3CreateForeignKey(Parse * pParse, /* Parsing context */
- ExprList * pFromCol, /* Columns in this table that
point to other table */
- Token * pTo, /* Name of the other table */
- ExprList * pToCol, /* Columns in the other table */
- int flags /* Conflict resolution algorithms. */
- )
+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)