On 7 Dec 2018, at 14:27, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:

On 07/12/2018 14:20, n.pettik wrote:

On 5 Dec 2018, at 23:35, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:

Vova, please, ignore. It was accidentally sent to you.

Nikita, please, review.

I didn’t receive mail since there were issues with our mailing list,
so I looked at patch-set on GitHub.

On 05/12/2018 02:47, Roman Khabibov wrote:

Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3580-err-msg-pathjoin
Issue: https://github.com/tarantool/tarantool/issues/3580
Roman Khabibov (2):
  sql: add CHAR description without length

    Fix an ability to describe CHAR without specifying the

Not ‘fix’ but rather ‘add’. Otherwise, it sounds wierd.

    length as it is allowed by standard. It was
    accidentally broken by this commit:
    7752cdfd31f9956a4d6bb0f306c561a0ac73e7ab
         Needed for #3616

It is not needed for #3616 - this commit and issue are not connected.
Issue disappeared after static types were introduced. It is ok that
you added test case on this issue, but these two problems are not related.
Consider refactoring:
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index f5e86fba1..d56fce451 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1485,26 +1485,22 @@ typedef(A) ::= DATE . { A.type = AFFINITY_REAL; }
 typedef(A) ::= TIME . { A.type = AFFINITY_REAL; }
 typedef(A) ::= DATETIME . { A.type = AFFINITY_REAL; }
 -%type char_len {int}
-typedef(A) ::= CHAR char_len(B) . {
+typedef(A) ::= CHAR . {
   A.type = AFFINITY_TEXT;
-  (void) B;
 }
 -%type vchar_len {int}
-typedef(A) ::= VARCHAR vchar_len(B) . {
-  A.type = AFFINITY_TEXT;
+char_len(A) ::= LP INTEGER(B) RP . {
+  (void) A;
   (void) B;
 }
 -char_len(A) ::=  . {
-  (void) A;
+typedef(A) ::= CHAR char_len(B) . {
+  A.type = AFFINITY_TEXT;
+  (void) B;
 }
 -char_len(A) ::=  vchar_len(A) .
-
-vchar_len(A) ::= LP INTEGER(B) RP . {
-  (void) A;
+typedef(A) ::= VARCHAR char_len(B) . {
+  A.type = AFFINITY_TEXT;
   (void) B;
 }
In this case code looks a bit more straightforward IMHO.

I've tried this refactoring too but faced with
grammar 'features'. You can not define two rules
starting with the same prefix. Did you test it? Maybe
I was wrong?