On 05/19/2018 12:37 AM, Vladislav Shpilevoy wrote:
> Hello. Thanks for the fixes! You did a huge work fixing the tests!
>
>>      >
>>      > -%include {
>>      > -  /* A routine to convert a binary TK_IS or TK_ISNOT 
>> expression into a
>>      > -  ** unary TK_ISNULL or TK_NOTNULL expression. */
>>
>>     3. I still can grep ISNULL token. NOTNULL too. They must be 
>> removed completely, but
>>     very accurately. For example, TK_NULL, TK_ISNULL, TK_NOTNULL are 
>> not tokens, they
>>     are identifiers for 'NULL', 'IS NULL', and 'IS NOT NULL'. But 
>> strings 'ISNULL' and
>>     'NOTNULL' must be removed. For example, look 
>> extra/mkkeywordhash.c:196. Here all the
>>     tokens are stored and their identifiers, so "NOTNULL" is 
>> reflected into TK_NOTNULL.
>>     We must remove "NOTNULL". Same in parse.y:137, 213.
>>
>>     In treeview.c:475 NOTNULL must be replaced with "NOT NULL". Same 
>> in vdbeaux.c:1508.
>>
>>     Same about ISNULL.
>>
>> Partially done. Ones in parse.y used to generate identifiers to 
>> operations 'IS NULL' and 'IS NOT NULL'
>
> I see, but they now are unused by parser. Please, remove them. The 
> parser uses not "NOTNULL" or
> "ISNULL" - it uses TK_ISNULL and TK_NOTNULL. Look at addopcodes.sh how 
> to add TK_... constants
> with no adding them to parser.
>
> In the final patch you must not be able to grep "ISNULL" or "NOTNULL" 
> strings anywhere.
>
> By removal of these parser-unused things you will make the parser 
> slightly faster.
>
>
> The rest of the patch is ok.
Done.