Hi! I was asked by Kirill to review this patchset, so here my
review turns up. See 3 comments below.

Many thanks for provided review, I really appreciate it.

On 17/09/2018 23:32, Nikita Pettik wrote:
Branch: https://github.com/tarantool/tarantool/tree/np/sql-static-types
Issues:
https://github.com/tarantool/tarantool/issues/2494
https://github.com/tarantool/tarantool/issues/2620
https://github.com/tarantool/tarantool/issues/2645

1. Why 2645? I do not see where do you check types
in the parser. Also, Kirill said that it is relocated
to 2.1.1/2.2.0 (I do not remember exact milestone).

I thought this ticket not about real static typing (i.e. implementing
all type checks and conversions during query compilation), but
concerning simple banning of things like:

CREATE TABLE t1 (id NOT_REALLY_INT PRIMARY KEY);

In other words, patch parser to accept only allowed by ANSI SQL types.
Correct me if I’m wrong.

https://github.com/tarantool/tarantool/issues/3018
https://github.com/tarantool/tarantool/issues/3104
https://github.com/tarantool/tarantool/issues/3459

2. Is 3459 fixed? I do not see its mention in the
commits.

Yep, it is automatically fixed within  #3104 i.e. when real type
hits space format, index type is displayed correctly.
Also, I put comment concerning this to the issue description.
I’ve put this mention to appropriate commit message to close issue.

This patch-set forces type specification for tables' columns.
From now, our grammar doesn't support typeless columns.
Allowed types are:
FLOAT, REAL, DOUBLE, NUMERIC, DECIMAL, INTEGER, TEXT, CHAR, VARCHAR,
BLOB, TIME, DATE, DATETIME.
Despite diversity of types, they are translated only to four types
from Tarantool's core: NUMBER, STRING, INT, SCALAR. Moreover,
restrictions such as length of CHAR type or precision of numeric type
are not involved now. Internally, AFFINITY is still used.
Fourth patch attempts at implementing implicit types conversions
to make it look closer to other DBs.
This patch-set refactors almost all tests in sql-tap/ suite.
Since number of tests is quite LARGE, some of them may contain
mistakes or strange behaviour. Initial plan was to automatically
spread INT type among all tables and fix failing tests. It turns out,
that not all tests can be converted: for instance, selectA.test.lua
originally almost in all subtests (more than 10k) inserts to one

3. selectA.test.lua consists of 2536 lines. Why 10k? What is a
problem to convert it manually?

Ok, mb I meant 10k in general…But anyway, selectA is unlikely
to be fixed, it is rather about completely rewrite this test.
And to make it fair, we should manually check each select’s result...
If you don’t insist, I will better open issue to resurrect this test.

Finally, I’ve rebased patch-set on fresh 2.0 (i.e. on top of DD integration
patches) and Travis is completely green.