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 20:24:17 +0300 [thread overview] Message-ID: <65a5628f-17db-c7c3-a0e5-8e81eb98da30@tarantool.org> (raw) In-Reply-To: <f7459a0b-a2a7-0ddb-318f-64a24e868fc5@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
next prev parent reply other threads:[~2021-08-06 17:24 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 2021-08-06 17:24 ` Safin Timur via Tarantool-patches [this message] 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=65a5628f-17db-c7c3-a0e5-8e81eb98da30@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