Re: [Tarantool-patches] [PATCH v3 2/9] lua: built-in module datetime

[Safin Timur via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On 06.08.2021 16:00, Safin Timur via Tarantool-patches wrote:
> 
> On 06.08.2021 4:30, Oleg Babin wrote:
>>
>> On 06.08.2021 03:23, Safin Timur wrote:
>>>
>>> i.e. whenever we see not datetime or interva value datetime_cmp 
>>> return nil, and all comparisons always return false. Please give me 
>>> know if that's incorrect scenario and we should raise exception. This 
>>> was the way Oleg Bain has requested it to behave. (return false for 
>>> bogus data)
>>>
>> Hi! Seems you understood me wrong. It was only about equal comparison.
>>
>> ```
>>
>> tarantool> uuid.new() == newproxy()
>> ---
>> - false
>> ...
>>
>> tarantool> uuid.new() > newproxy()
>> ---
>> - error: '[string "return require(''uuid'').new() > newproxy()"]:1: 
>> incorrect value
>>      to convert to uuid as 1 argument'
>> ...
>>
>> ```
>>
>>
>> However, currently I see that decimal have a bit different behaviour 
>> and raises always:
>>
>> ```
>>
>> tarantool> decimal.new(1) == newproxy()
>> ---
>> - error: '[string "return decimal.new(1) == newproxy()"]:1: expected 
>> decimal, number
>>      or string as 2 argument'
>> ...
>>
>> tarantool> decimal.new(1) > newproxy()
>> ---
>> - error: '[string "return decimal.new(1) > newproxy()"]:1: expected 
>> decimal, number
>>      or string as 1 argument'
>> ...
>>
>> ```
>>
>>
>> IMO, the first case is better because we can check that two lua 
>> built-in types
>>
>> are equal but can't compare them.
>>
>> ```
>>
>> tarantool> {} == 'string'
>> ---
>> - false
>> ...
>>
>> tarantool> {} > 'string'
>> ---
>> - error: '[string "return {} > ''string''"]:1: attempt to compare 
>> string with table'
>> ...
>>
>> ```
>>
>>
> 
> Now, that looks more or less consistent, will do it such!
> 
> ```
> tarantool> date = require 'datetime'
> ---
> ...
> 
> tarantool> T = date('1970-01-01')
> ---
> ...
> 
> tarantool> T == false
> ---
> - false
> ...
> 
> tarantool> T == true
> ---
> - false
> ...
> 
> tarantool> T < false
> ---
> - error: '[string "return T < false"]:1: incompatible types for comparison'
> ...
> 
> tarantool> T <= 'string'
> ---
> - error: '[string "return T <= ''string''"]:1: incompatible types for 
> comparison'
> ...
> ```
> 
> Patch is simple though
> ==============================================================
> diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua
> index 0bc86c9f3..509a2981f 100644
> --- a/src/lua/datetime.lua
> +++ b/src/lua/datetime.lua
> @@ -245,12 +245,12 @@ end
> 
>   local function datetime_lt(lhs, rhs)
>       local rc = datetime_cmp(lhs, rhs)
> -    return rc ~= nil and rc < 0 or false
> +    return rc ~= nil and rc < 0 or error('incompatible types for 
> comparison', 2)
>   end
> 
>   local function datetime_le(lhs, rhs)
>       local rc = datetime_cmp(lhs, rhs)
> -    return rc ~= nil and rc <= 0 or false
> +    return rc ~= nil and rc <= 0 or  error('incompatible types for 
> comparison', 2)
>   end
> 
>   local function datetime_serialize(self)
> ==============================================================

Well, actually that was incorrect version (it was raising error if we 
have compatible types but comparison failed), condition should look 
different, but idea you saw correct:

============================================================
local function datetime_eq(lhs, rhs)
     local rc = datetime_cmp(lhs, rhs)
     return rc ~= nil and rc == 0
end

local function datetime_lt(lhs, rhs)
     local rc = datetime_cmp(lhs, rhs)
     return rc == nil and error('incompatible types for comparison', 2) or
            rc < 0
end

local function datetime_le(lhs, rhs)
     local rc = datetime_cmp(lhs, rhs)
     return rc == nil and error('incompatible types for comparison', 2) or
            rc <= 0
end
============================================================
> 
> Do we want to use that approach here?
> 
> Thanks,
> Timur