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
next prev parent 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