[tarantool-patches] Re: [PATCH 0/3 v2] sql: add test for indexed char in sub subquery

["n.pettik" <korablev@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2581 bytes --]



> 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?

Yep, I tested - it works.


[-- Attachment #2: Type: text/html, Size: 8500 bytes --]