Tarantool development patches archive
 help / color / mirror / Atom feed
From: Safin Timur via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Oleg Babin <olegrok@tarantool.org>,
	Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v3 2/9] lua: built-in module datetime
Date: Fri, 6 Aug 2021 16:00:35 +0300	[thread overview]
Message-ID: <f7459a0b-a2a7-0ddb-318f-64a24e868fc5@tarantool.org> (raw)
In-Reply-To: <331a5d04-d756-fac0-fa07-cac594fa7ed8@tarantool.org>


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)
==============================================================

Do we want to use that approach here?

Thanks,
Timur

  reply	other threads:[~2021-08-06 13:00 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02  0:40 [Tarantool-patches] [PATCH v3 0/9] Initial datetime support Timur Safin via Tarantool-patches
2021-08-02  0:40 ` [Tarantool-patches] [PATCH v3 1/9] build: add Christian Hansen c-dt to the build Timur Safin via Tarantool-patches
2021-08-04 23:58   ` Vladislav Shpilevoy via Tarantool-patches
2021-08-05  8:55     ` Safin Timur via Tarantool-patches
2021-08-08 14:34       ` Vladislav Shpilevoy via Tarantool-patches
2021-08-02  0:40 ` [Tarantool-patches] [PATCH v3 2/9] lua: built-in module datetime Timur Safin via Tarantool-patches
2021-08-04 23:58   ` Vladislav Shpilevoy via Tarantool-patches
2021-08-06  0:23     ` Safin Timur via Tarantool-patches
2021-08-06  1:30       ` Oleg Babin via Tarantool-patches
2021-08-06 13:00         ` Safin Timur via Tarantool-patches [this message]
2021-08-06 17:24           ` Safin Timur via Tarantool-patches
2021-08-08 11:26       ` Vladislav Shpilevoy via Tarantool-patches
2021-08-08 16:35         ` Safin Timur via Tarantool-patches
2021-08-10 12:20           ` Vladislav Shpilevoy via Tarantool-patches
2021-08-10 12:21             ` Igor Munkin via Tarantool-patches
2021-08-12 20:47               ` Safin Timur via Tarantool-patches
2021-08-15 20:52                 ` Safin Timur via Tarantool-patches
2021-08-06  0:26   ` Safin Timur via Tarantool-patches
2021-08-08 14:34   ` Vladislav Shpilevoy via Tarantool-patches
2021-08-08 16:47     ` Safin Timur via Tarantool-patches
2021-08-02  0:40 ` [Tarantool-patches] [PATCH v3 3/9] lua, datetime: datetime tests Timur Safin via Tarantool-patches
2021-08-06  0:25   ` Safin Timur via Tarantool-patches
2021-08-02  0:41 ` [Tarantool-patches] [PATCH v3 4/9] lua, datetime: display datetime Timur Safin via Tarantool-patches
2021-08-02  0:41 ` [Tarantool-patches] [PATCH v3 5/9] box, datetime: messagepack support for datetime Timur Safin via Tarantool-patches
2021-08-03 13:38   ` Timur Safin via Tarantool-patches
2021-08-02  0:41 ` [Tarantool-patches] [PATCH v3 6/9] box, datetime: datetime comparison for indices Timur Safin via Tarantool-patches
2021-08-03 12:02   ` Serge Petrenko via Tarantool-patches
2021-08-03 12:59     ` Timur Safin via Tarantool-patches
2021-08-04 10:12     ` Timur Safin via Tarantool-patches
2021-08-02  0:41 ` [Tarantool-patches] [PATCH v3 7/9] lua, datetime: time intervals support Timur Safin via Tarantool-patches
2021-08-02  0:41 ` [Tarantool-patches] [PATCH v3 8/9] datetime: changelog for datetime module Timur Safin via Tarantool-patches
2021-08-02  0:41 ` [Tarantool-patches] [PATCH v3 9/9] lua, box, datetime: rename struct datetime_t Timur Safin via Tarantool-patches
2021-08-06  0:27   ` Safin Timur via Tarantool-patches
2021-08-03 21:23 ` [Tarantool-patches] [PATCH v3 1/2] datetime: update tests for macosx Timur Safin via Tarantool-patches
2021-08-06  0:28   ` Safin Timur via Tarantool-patches
2021-08-03 21:23 ` [Tarantool-patches] [PATCH v3 2/2] lua, datetime: introduce ctime, strftime wrappers Timur Safin via Tarantool-patches
2021-08-06  0:30   ` Safin Timur via Tarantool-patches
2021-08-03 21:26 ` [Tarantool-patches] [PATCH v3 0/9] Initial datetime support Timur Safin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f7459a0b-a2a7-0ddb-318f-64a24e868fc5@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=olegrok@tarantool.org \
    --cc=tsafin@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v3 2/9] lua: built-in module datetime' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox