From: Mergen Imeev <imeevma@tarantool.org>
To: Nikita Pettik <korablev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations
Date: Sun, 27 Sep 2020 12:11:52 +0300 [thread overview]
Message-ID: <20200927091152.GA60576@tarantool.org> (raw)
In-Reply-To: <20200917133648.GF10599@tarantool.org>
Привет! Спасибо за ревью. Мои ответы и новый патч ниже.
On Thu, Sep 17, 2020 at 01:36:48PM +0000, Nikita Pettik wrote:
> On 21 Aug 15:34, Mergen Imeev wrote:
> > Hi! Thank you for the review. My answer and new patch below.
> >
> > On Fri, Aug 21, 2020 at 09:21:30AM +0000, Nikita Pettik wrote:
> > > On 21 Aug 11:40, imeevma@tarantool.org wrote:
> > > > This patch removes the implicit conversion from STRING to INTEGER from
> > > > bitwise operations. However, DOUBLE can still be implicitly converted to
> > > > INTEGER.
> > >
> > > I see no test involving doubles in bitwise operations. Does it make
> > > any sense at all?
> > Even if it doesn't, it is legal to implicitly convert DOUBLE to INTEGER.
> >
> > However, when I tried to add tests for this case, I found an error in my patch.
> > I re-made patch. Now in these opcodes we convert MEM to INTEGER, which I tried
> > to avoid in previous patch. I did this to fix a bug where result of the
> > operation is DOUBLE if one of the operands is DOUBLE. It didn't help, the
> > result still has DOUBLE type. I decided to left conversion since it looks right
> > here because all operands must be INTEGERS. This wouldn't work for arithmetic
> > operations though.
>
> Не понял ничего..Как эта штука должна работать??
> box.execute([[SELECT 3.5 | 1.3;]])
> metadata:
> name: COLUMN_1
> type: double
> rows:
> [3] -- WTF
>
В соответствии с правилами implicit cast between numeric values. В любом случае,
убрал возможность делать значения типа DOUBLE операндами битовых операций.
> Давай по-честному выдавать ошибку, когда в битовые операции суем не инты.
Сделал.
>
> > From 3515ada4b363062cf9caa5d550ea40770e8a5e65 Mon Sep 17 00:00:00 2001
> > From: Mergen Imeev <imeevma@gmail.com>
> > Date: Tue, 18 Aug 2020 18:18:59 +0300
> > Subject: [PATCH] sql: remove implicit cast in bitwise operations
> >
> > This patch removes the implicit conversion from STRING to INTEGER from
> > bitwise operations. However, DOUBLE can still be implicitly converted to
> > INTEGER.
> >
> > Follow-up #3809
> >
Новый патч:
From a630c39d486769d1a684e56b15bbf5107e7bef66 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Tue, 18 Aug 2020 18:18:59 +0300
Subject: [PATCH] sql: remove implicit cast in bitwise operations
This patch removes the implicit conversion from STRING to INTEGER from
bitwise operations. Only integer values allowed in bitwise operations.
Follow-up #3809
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index c0143a6b1..5d1ab8c94 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1989,23 +1989,26 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */
pIn1 = &aMem[pOp->p1];
pIn2 = &aMem[pOp->p2];
+ enum mp_type type1 = mem_mp_type(pIn1);
+ enum mp_type type2 = mem_mp_type(pIn2);
pOut = vdbe_prepare_null_out(p, pOp->p3);
- if ((pIn1->flags | pIn2->flags) & MEM_Null) {
+ if (type1 == MP_NIL || type2 == MP_NIL) {
/* Force NULL be of type INTEGER. */
pOut->field_type = FIELD_TYPE_INTEGER;
break;
}
- bool unused;
- if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) {
+ if (type2 != MP_INT && type2 != MP_UINT) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(pIn2), "integer");
goto abort_due_to_error;
}
- if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) {
+ if (type1 != MP_INT && type1 != MP_UINT) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
sql_value_to_diag_str(pIn1), "integer");
goto abort_due_to_error;
}
+ iA = pIn2->u.i;
+ iB = pIn1->u.i;
op = pOp->opcode;
if (op==OP_BitAnd) {
iA &= iB;
@@ -2621,19 +2624,18 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */
*/
case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */
pIn1 = &aMem[pOp->p1];
+ enum mp_type type = mem_mp_type(pIn1);
pOut = vdbe_prepare_null_out(p, pOp->p2);
/* Force NULL be of type INTEGER. */
pOut->field_type = FIELD_TYPE_INTEGER;
- if ((pIn1->flags & MEM_Null)==0) {
- int64_t i;
- bool is_neg;
- if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) {
- diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
- sql_value_to_diag_str(pIn1), "integer");
- goto abort_due_to_error;
- }
- mem_set_i64(pOut, ~i);
+ if (type == MP_NIL)
+ break;
+ if (type != MP_INT && type != MP_UINT) {
+ diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+ sql_value_to_diag_str(pIn1), "integer");
+ goto abort_due_to_error;
}
+ mem_set_i64(pOut, ~pIn1->u.i);
break;
}
diff --git a/test/sql/types.result b/test/sql/types.result
index caedbf409..a79818b30 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -2846,3 +2846,284 @@ box.execute([[SELECT 1 - '2';]])
- null
- 'Type mismatch: can not convert 2 to numeric'
...
+--
+-- Make sure there is no implicit string-to-number conversion in bitwise
+-- operations.
+--
+box.execute([[SELECT '1' | 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT '1' & 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT '1' << 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT '1' >> 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT ~'1';]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT 1 | '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to integer'
+...
+box.execute([[SELECT 1 & '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to integer'
+...
+box.execute([[SELECT 1 << '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to integer'
+...
+box.execute([[SELECT 1 >> '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to integer'
+...
+--
+-- Make sure that only values of type INTEGER can be operands of bitwise
+-- operations.
+--
+box.execute([[SELECT 3 | 1;]])
+---
+- metadata:
+ - name: COLUMN_1
+ type: integer
+ rows:
+ - [3]
+...
+box.execute([[SELECT 3 | 1.1;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT 3 | '1';]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT 3 | true;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT 3 | X'31';]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT 1 | 3;]])
+---
+- metadata:
+ - name: COLUMN_1
+ type: integer
+ rows:
+ - [3]
+...
+box.execute([[SELECT 1.1 | 3;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT '1' | 3;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT true | 3;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT X'31' | 3;]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT 3 & 1;]])
+---
+- metadata:
+ - name: COLUMN_1
+ type: integer
+ rows:
+ - [1]
+...
+box.execute([[SELECT 3 & 1.1;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT 3 & '1';]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT 3 & true;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT 3 & X'31';]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT 1.1 & 3;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT '1' & 3;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT true & 3;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT X'31' & 3;]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT 3 << 1;]])
+---
+- metadata:
+ - name: COLUMN_1
+ type: integer
+ rows:
+ - [6]
+...
+box.execute([[SELECT 3 << 1.1;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT 3 << '1';]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT 3 << true;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT 3 << X'31';]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT 1.1 << 3;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT '1' << 3;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT true << 3;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT X'31' << 3;]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT 3 >> 1;]])
+---
+- metadata:
+ - name: COLUMN_1
+ type: integer
+ rows:
+ - [1]
+...
+box.execute([[SELECT 3 >> 1.1;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT 3 >> '1';]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT 3 >> true;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT 3 >> X'31';]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT 1.1 >> 3;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT '1' >> 3;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT true >> 3;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT X'31' >> 3;]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT ~1;]])
+---
+- metadata:
+ - name: COLUMN_1
+ type: integer
+ rows:
+ - [-2]
+...
+box.execute([[SELECT ~1.1;]])
+---
+- null
+- 'Type mismatch: can not convert 1.1 to integer'
+...
+box.execute([[SELECT ~'1';]])
+---
+- null
+- 'Type mismatch: can not convert 1 to integer'
+...
+box.execute([[SELECT ~true;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to integer'
+...
+box.execute([[SELECT ~X'31';]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index 844a6b670..bb922e78f 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -638,3 +638,68 @@ box.execute([[SELECT 1 % '2';]])
box.execute([[SELECT 1 * '2';]])
box.execute([[SELECT 1 / '2';]])
box.execute([[SELECT 1 - '2';]])
+
+--
+-- Make sure there is no implicit string-to-number conversion in bitwise
+-- operations.
+--
+box.execute([[SELECT '1' | 2;]])
+box.execute([[SELECT '1' & 2;]])
+box.execute([[SELECT '1' << 2;]])
+box.execute([[SELECT '1' >> 2;]])
+box.execute([[SELECT ~'1';]])
+box.execute([[SELECT 1 | '2';]])
+box.execute([[SELECT 1 & '2';]])
+box.execute([[SELECT 1 << '2';]])
+box.execute([[SELECT 1 >> '2';]])
+
+--
+-- Make sure that only values of type INTEGER can be operands of bitwise
+-- operations.
+--
+box.execute([[SELECT 3 | 1;]])
+box.execute([[SELECT 3 | 1.1;]])
+box.execute([[SELECT 3 | '1';]])
+box.execute([[SELECT 3 | true;]])
+box.execute([[SELECT 3 | X'31';]])
+box.execute([[SELECT 1 | 3;]])
+box.execute([[SELECT 1.1 | 3;]])
+box.execute([[SELECT '1' | 3;]])
+box.execute([[SELECT true | 3;]])
+box.execute([[SELECT X'31' | 3;]])
+
+box.execute([[SELECT 3 & 1;]])
+box.execute([[SELECT 3 & 1.1;]])
+box.execute([[SELECT 3 & '1';]])
+box.execute([[SELECT 3 & true;]])
+box.execute([[SELECT 3 & X'31';]])
+box.execute([[SELECT 1.1 & 3;]])
+box.execute([[SELECT '1' & 3;]])
+box.execute([[SELECT true & 3;]])
+box.execute([[SELECT X'31' & 3;]])
+
+box.execute([[SELECT 3 << 1;]])
+box.execute([[SELECT 3 << 1.1;]])
+box.execute([[SELECT 3 << '1';]])
+box.execute([[SELECT 3 << true;]])
+box.execute([[SELECT 3 << X'31';]])
+box.execute([[SELECT 1.1 << 3;]])
+box.execute([[SELECT '1' << 3;]])
+box.execute([[SELECT true << 3;]])
+box.execute([[SELECT X'31' << 3;]])
+
+box.execute([[SELECT 3 >> 1;]])
+box.execute([[SELECT 3 >> 1.1;]])
+box.execute([[SELECT 3 >> '1';]])
+box.execute([[SELECT 3 >> true;]])
+box.execute([[SELECT 3 >> X'31';]])
+box.execute([[SELECT 1.1 >> 3;]])
+box.execute([[SELECT '1' >> 3;]])
+box.execute([[SELECT true >> 3;]])
+box.execute([[SELECT X'31' >> 3;]])
+
+box.execute([[SELECT ~1;]])
+box.execute([[SELECT ~1.1;]])
+box.execute([[SELECT ~'1';]])
+box.execute([[SELECT ~true;]])
+box.execute([[SELECT ~X'31';]])
next prev parent reply other threads:[~2020-09-27 9:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-21 8:40 [Tarantool-patches] [PATCH v1 0/2] sql: remove implicit cast from operations imeevma
2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations imeevma
2020-08-21 8:59 ` Nikita Pettik
2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations imeevma
2020-08-21 9:21 ` Nikita Pettik
2020-08-21 12:34 ` Mergen Imeev
2020-09-17 13:36 ` Nikita Pettik
2020-09-27 9:11 ` Mergen Imeev [this message]
2020-09-28 17:13 ` Nikita Pettik
2020-09-29 13:00 ` Mergen Imeev
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=20200927091152.GA60576@tarantool.org \
--to=imeevma@tarantool.org \
--cc=korablev@tarantool.org \
--cc=tarantool-patches@dev.tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations' \
/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