From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Serge Petrenko Message-Id: <940F74CC-9A3F-49CE-9B1D-A265D27B08A5@tarantool.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_17FAD15E-9942-477B-8894-90BDF9F8E81C" Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: Re: [tarantool-patches] Re: [PATCH] lua: fix decimal comparison with box.NULL Date: Tue, 27 Aug 2019 15:10:47 +0300 In-Reply-To: <20190827120448.GN13834@esperanza> References: <20190827112209.43581-1-sergepetrenko@tarantool.org> <20190827120448.GN13834@esperanza> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: --Apple-Mail=_17FAD15E-9942-477B-8894-90BDF9F8E81C Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > 27 =D0=B0=D0=B2=D0=B3. 2019 =D0=B3., =D0=B2 15:04, Vladimir Davydov = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0= ): >=20 > On Tue, Aug 27, 2019 at 02:22:09PM +0300, Serge Petrenko wrote: >> This problem is similar to the one fixed in commit >> 3c6c1cc96a1a510493c21c472565d4063e403ba2 (lua:fix decimal comparison >> with nil) >> We should handle box.NULL the same way. >>=20 >> Closes #4454 >> --- >> https://github.com/tarantool/tarantool/issues/4454 >> = https://github.com/tarantool/tarantool/tree/sp/gh-4454-decimal-cmp-null >>=20 >> src/lua/decimal.c | 3 ++- >> test/app/decimal.result | 30 ++++++++++++++++++++++++++++++ >> test/app/decimal.test.lua | 8 ++++++++ >> 3 files changed, 40 insertions(+), 1 deletion(-) >>=20 >> diff --git a/src/lua/decimal.c b/src/lua/decimal.c >> index 23e50ba68..8905a0b9d 100644 >> --- a/src/lua/decimal.c >> +++ b/src/lua/decimal.c >> @@ -235,7 +235,8 @@ static int >> ldecimal_eq(struct lua_State *L) >> { >> assert(lua_gettop(L) =3D=3D 2); >> - if (lua_isnil(L, 1) || lua_isnil(L, 2)) { >> + if (lua_isnil(L, 1) || lua_isnil(L, 2) || >> + luaL_isnull(L, 1) || luaL_isnull(L, 2)) { >> lua_pushboolean(L, false); >> return 1; >> } >> diff --git a/test/app/decimal.result b/test/app/decimal.result >> index 8251e13d8..90d0984b0 100644 >> --- a/test/app/decimal.result >> +++ b/test/app/decimal.result >> @@ -248,6 +248,36 @@ a <=3D nil >> | --- >> | - error: '[string "return a <=3D nil "]:1: attempt to compare = decimal with nil' >> | ... >> +-- and with box.NULL >> +-- >> +a =3D=3D box.NULL >> + | --- >> + | - false >> + | ... >> +a ~=3D box.NULL >> + | --- >> + | - true >> + | ... >> +a > box.NULL >> + | --- >> + | - error: '[string "return a > box.NULL "]:1: expected decimal, = number or string as >> + | 1 argument' >> + | ... >=20 > 1 argument? 1st argument >=20 >> +a < box.NULL >> + | --- >> + | - error: '[string "return a < box.NULL "]:1: expected decimal, = number or string as >> + | 2 argument' >> + | ... >=20 > 2 argument? 2nd argument >=20 > Why does the error message differ for the two seemingly equivalent > cases? What's "1 argument"? >=20 > Note, in case of comparison with nil the error message is different: >=20 > | - error: '[string "return a > nil "]:1: attempt to compare decimal = with nil' > | - error: '[string "return a < nil "]:1: attempt to compare decimal = with nil=E2=80=99 When you say =E2=80=985 < nil=E2=80=99 you get an error =E2=80=98attempt = to compare number with nil=E2=80=99 However, when you say =E2=80=985 < box.NULL=E2=80=99 you get attempt to = compare number with =E2=80=98void *=E2=80=99. So I tried to be consistent. Kind of. And just leave the message as it = is. We can say =E2=80=98attempt to compare decimal with box.NULL` or with `void *` if you wish. -- Serge Petrenko sergepetrenko@tarantool.org --Apple-Mail=_17FAD15E-9942-477B-8894-90BDF9F8E81C Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

27 = =D0=B0=D0=B2=D0=B3. 2019 =D0=B3., =D0=B2 15:04, Vladimir Davydov <vdavydov.dev@gmail.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0= =B0=D0=BB(=D0=B0):

On Tue, Aug = 27, 2019 at 02:22:09PM +0300, Serge Petrenko wrote:
This = problem is similar to the one fixed in commit
3c6c1cc96a1a510493c21c472565d4063e403ba2 (lua:fix decimal = comparison
with nil)
We should handle = box.NULL the same way.

Closes #4454
---
https://github.com/tarantool/tarantool/issues/4454
https://github.com/tarantool/tarantool/tree/sp/gh-4454-decimal-= cmp-null

src/lua/decimal.c =         | 3 ++-
test/app/decimal.result   | 30 = ++++++++++++++++++++++++++++++
test/app/decimal.test.lua | = 8 ++++++++
3 files changed, 40 insertions(+), 1 = deletion(-)

diff --git a/src/lua/decimal.c = b/src/lua/decimal.c
index 23e50ba68..8905a0b9d 100644
--- a/src/lua/decimal.c
+++ = b/src/lua/decimal.c
@@ -235,7 +235,8 @@ static int
ldecimal_eq(struct lua_State *L)
{
= assert(lua_gettop(L) =3D=3D 2);
- if = (lua_isnil(L, 1) || lua_isnil(L, 2)) {
+ if = (lua_isnil(L, 1) || lua_isnil(L, 2) ||
+     luaL_isnull= (L, 1) || luaL_isnull(L, 2)) {
= lua_pushboolean(L, false);
return = 1;
}
diff --git a/test/app/decimal.result = b/test/app/decimal.result
index 8251e13d8..90d0984b0 = 100644
--- a/test/app/decimal.result
+++ = b/test/app/decimal.result
@@ -248,6 +248,36 @@ a <=3D = nil
| ---
| - error: '[string = "return a <=3D nil "]:1: attempt to compare decimal with nil'
| ...
+-- and with box.NULL
+--
+a =3D=3D box.NULL
+ | ---
+ | - false
+ | ...
+a ~=3D = box.NULL
+ | ---
+ | - true
+ = | ...
+a > box.NULL
+ | ---
+= | - error: '[string "return a > box.NULL "]:1: expected decimal, = number or string as
+ |     1 = argument'
+ | ...

1 argument?

1st = argument

+a = < box.NULL
+ | ---
+ | - error: '[string = "return a < box.NULL "]:1: expected decimal, number or string as
+ | 2 argument'
+ | = ...

2 argument?

2nd argument

Why does the error message differ for the two seemingly = equivalent
cases? What's = "1 argument"?

Note, in case = of comparison with nil the error message is different:

| - error: '[string "return a = > nil "]:1: attempt to compare decimal with nil'
| - error: '[string "return a = < nil "]:1: attempt to compare decimal with = nil=E2=80=99

When you = say =E2=80=985 < nil=E2=80=99 you get an error =E2=80=98attempt to = compare number with nil=E2=80=99

However, when you say =E2=80=98= 5 < box.NULL=E2=80=99 you get attempt to compare = number

with =E2=80=98void *=E2=80=99.

So I tried to be consistent. Kind of. And just = leave the message as it is.

We can say =E2=80=98attempt to = compare decimal with box.NULL`

or with `void *` if you = wish.

--

Serge = Petrenko

sergepetrenko@tarantool.org

= --Apple-Mail=_17FAD15E-9942-477B-8894-90BDF9F8E81C--