[tarantool-patches] Re: [PATCH 0/5] Move FK constraints to server

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hello. Thanks for the patchset! See 5 possible
typos below.

On 13/07/2018 05:04, Nikita Pettik wrote:
> Branch: https://github.com/tarantool/tarantool/commits/np/move-fk-to-server
> Issue: https://github.com/tarantool/tarantool/issues/3271
> 
> The aim of this patch-set is to move foreign key constraint to server
> and them be closer to ANSI specification.

1. Them -> then?

> 
> First patch is preliminary and enables additional restrictions
> for FK constraints (ban opportunity to drop space referenced space,

2. 'Space referenced space'? Can not parse.

> create FK referencing VIEW etc).
> 
> In original SQLite FK constraints may appear only during CREATE TABLE
> statement. Thus, it was enough to hold string of CREATE TABLE statement
> and reparse it once on instance loading. This approach defers all
> resolutions until FK usage. For instance:
> 
> CREATE TABLE t1(id PRIMARY KEY REFERENCES t2);
> CREATE TABLE t2(id PRIMARY KEY);
> 
> We decided to use another approach - where FK constraints are always consistent
> and all DD links are kept up. For instance, if we attempted to satisfy all
> restrictions using SQLite schema - we wouldn't be able to create circular
> dependencies. To support circular dependecies, we must allow to create them

3. dependecies -> dependencies.

> after space itself. In turn, to create FK constraints outside CREATE STATEMENT,
> we must persist them.  To implement these steps, firstly _fk_constraint system
> space is added - it contains tuples describing FK. Then, separate SQL statement
> <ALTER TABLE name ADD CONSTRAINT ...> is introduced which processes insertion
> and deletion from this space. Finally, FK processing has been refactored to
> rely on new DD in server (struct fkey and struct fkey_def). It seems that
> perfomance of FK handling has become a little bit better: now we don't need

4. perfomance -> performance.

> find suitable index on each FK invocation - its id is held into FK struct
> itself.
> 
> The last patch is simple follow-up which removes obsolete define guard
> for FK constraints.
> 
> Nikita Pettik (5):
>    sql: prohibit creation of FK on unexisting tables

5. unexisting -> non-existing? But I am not so good in English as you,
so maybe I am wrong.

>    schema: add new system space for FK constraints
>    sql: introduce ADD CONSTRAINT statement
>    sql: display error on FK creation and drop failure
>    sql: remove SQLITE_OMIT_FOREIGN_KEY define guard
> 
>   extra/mkkeywordhash.c                |    9 +-
>   src/box/CMakeLists.txt               |    1 +
>   src/box/alter.cc                     |  432 +++++++++++++-
>   src/box/alter.h                      |    1 +
>   src/box/bootstrap.snap               |  Bin 1704 -> 1798 bytes
>   src/box/errcode.h                    |    6 +
>   src/box/fkey.c                       |   69 +++
>   src/box/fkey.h                       |  169 ++++++
>   src/box/lua/schema.lua               |    6 +
>   src/box/lua/space.cc                 |    2 +
>   src/box/lua/upgrade.lua              |   16 +
>   src/box/schema.cc                    |   16 +
>   src/box/schema_def.h                 |   14 +
>   src/box/space.c                      |    2 +
>   src/box/space.h                      |    3 +
>   src/box/sql.c                        |   24 +
>   src/box/sql/alter.c                  |    7 -
>   src/box/sql/build.c                  |  601 ++++++++++++++-----
>   src/box/sql/callback.c               |    2 -
>   src/box/sql/delete.c                 |    6 +-
>   src/box/sql/expr.c                   |   10 +-
>   src/box/sql/fkey.c                   | 1077 ++++++++++------------------------
>   src/box/sql/insert.c                 |   21 +-
>   src/box/sql/main.c                   |    5 -
>   src/box/sql/parse.y                  |   37 +-
>   src/box/sql/pragma.c                 |  242 +-------
>   src/box/sql/pragma.h                 |   13 -
>   src/box/sql/prepare.c                |    5 +
>   src/box/sql/sqliteInt.h              |  185 +++---
>   src/box/sql/status.c                 |    5 +-
>   src/box/sql/tarantoolInt.h           |   13 +
>   src/box/sql/trigger.c                |   24 +-
>   src/box/sql/update.c                 |    4 +-
>   src/box/sql/vdbe.c                   |   51 +-
>   src/box/sql/vdbeInt.h                |    4 -
>   src/box/sql/vdbeaux.c                |    4 -
>   test/box/access_misc.result          |    5 +
>   test/box/access_sysview.result       |    6 +-
>   test/box/alter.result                |    5 +-
>   test/box/misc.result                 |    4 +
>   test/engine/iterator.result          |    2 +-
>   test/sql-tap/alter.test.lua          |    2 +-
>   test/sql-tap/alter2.test.lua         |  219 +++++++
>   test/sql-tap/fkey1.test.lua          |   65 +-
>   test/sql-tap/fkey2.test.lua          |  148 ++---
>   test/sql-tap/fkey3.test.lua          |   19 +-
>   test/sql-tap/fkey4.test.lua          |    2 +-
>   test/sql-tap/orderby1.test.lua       |    6 +-
>   test/sql-tap/suite.ini               |    1 +
>   test/sql-tap/table.test.lua          |   19 +-
>   test/sql-tap/tkt-b1d3a2e531.test.lua |    6 +-
>   test/sql-tap/triggerC.test.lua       |    2 +-
>   test/sql-tap/whereG.test.lua         |    4 +-
>   test/sql-tap/with1.test.lua          |    2 +-
>   test/sql/foreign-keys.result         |  316 ++++++++++
>   test/sql/foreign-keys.test.lua       |  144 +++++
>   56 files changed, 2534 insertions(+), 1529 deletions(-)
>   create mode 100644 src/box/fkey.c
>   create mode 100644 src/box/fkey.h
>   create mode 100755 test/sql-tap/alter2.test.lua
>   create mode 100644 test/sql/foreign-keys.result
>   create mode 100644 test/sql/foreign-keys.test.lua
>