[Tarantool-patches] [PATCH v5 21/52] sql: introduce bitwise operations for MEM

[Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thank you for the review! My answers and new patch below.


On 30.03.2021 02:03, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> On 23.03.2021 10:35, Mergen Imeev via Tarantool-patches wrote:
>> This patch introduces mem_bitwise(). Function mem_bitwise() executes
>> bitwise operations with two operands and writes the result to the third
>> MEM.
>>
>> Part of #5818
>> ---
>>  src/box/sql/mem.c  | 51 +++++++++++++++++++++++++++++++++++++++++++
>>  src/box/sql/mem.h  |  3 +++
>>  src/box/sql/vdbe.c | 54 +++-------------------------------------------
>>  3 files changed, 57 insertions(+), 51 deletions(-)
>>
>> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
>> index 8119644ed..2b455e39f 100644
>> --- a/src/box/sql/mem.c
>> +++ b/src/box/sql/mem.c
>> @@ -556,6 +556,57 @@ mem_arithmetic(const struct Mem *left, const struct Mem *right,
>>    return 0;
>>  }
>>  
>> +int
>> +mem_bitwise(struct Mem *left, struct Mem *right, struct Mem *result, int op)
>
> Would be better to split it into separate functions. Also why is OP_BitNot
> separated? How is it much different from, say, OP_BitAnd?
>
Done. I splitted this function. Also, I moved function mem_bit_not() to this
patch.

> Besides, having OP_BitNot in mem.c would allow to make sqlVdbeIntValue static
> inside mem.c.
I didn't make this function static since I will remove it in a few patches.


New patch:

commit cc2e656974210a7f4f8e3c5402adf1bd90c5576f
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Sun Mar 14 22:20:11 2021 +0300

    sql: introduce bitwise operations for MEM
    
    This patch introduces mem_bit_and(), mem_bit_or(), mem_shift_left(),
    mem_shift_right(), and mem_bit_not(), which perform bitwise operations
    on MEM.
    
    Part of #5818

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index eee72a7fe..aeb801c7c 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -624,6 +624,115 @@ mem_rem(const struct Mem *left, const struct Mem *right, struct Mem *result)
  return 0;
 }
 
+static int
+bitwise_prepare(const struct Mem *left, const struct Mem *right,
+   int64_t *a, int64_t *b)
+{
+ bool unused;
+ if (sqlVdbeIntValue(left, a, &unused) != 0) {
+   diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(left),
+      "integer");
+   return -1;
+ }
+ if (sqlVdbeIntValue(right, b, &unused) != 0) {
+   diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(right),
+      "integer");
+   return -1;
+ }
+ return 0;
+}
+
+int
+mem_bit_and(const struct Mem *left, const struct Mem *right, struct Mem *result)
+{
+ if (is_result_null(left, right, result, FIELD_TYPE_INTEGER))
+   return 0;
+ int64_t a;
+ int64_t b;
+ if (bitwise_prepare(left, right, &a, &b) != 0)
+   return -1;
+ result->u.i = a & b;
+ result->flags = result->u.i < 0 ? MEM_Int : MEM_UInt;
+ return 0;
+}
+
+int
+mem_bit_or(const struct Mem *left, const struct Mem *right, struct Mem *result)
+{
+ if (is_result_null(left, right, result, FIELD_TYPE_INTEGER))
+   return 0;
+ int64_t a;
+ int64_t b;
+ if (bitwise_prepare(left, right, &a, &b) != 0)
+   return -1;
+ result->u.i = a | b;
+ result->flags = result->u.i < 0 ? MEM_Int : MEM_UInt;
+ return 0;
+}
+
+int
+mem_shift_left(const struct Mem *left, const struct Mem *right,
+        struct Mem *result)
+{
+ if (is_result_null(left, right, result, FIELD_TYPE_INTEGER))
+   return 0;
+ int64_t a;
+ int64_t b;
+ if (bitwise_prepare(left, right, &a, &b) != 0)
+   return -1;
+ if (b <= -64)
+   result->u.i = a >= 0 ? 0 : -1;
+ else if (b < 0)
+   result->u.i = a >> -b;
+ else if (b > 64)
+   result->u.i = 0;
+ else
+   result->u.i = a << b;
+ result->flags = result->u.i < 0 ? MEM_Int : MEM_UInt;
+ return 0;
+}
+
+int
+mem_shift_right(const struct Mem *left, const struct Mem *right,
+   struct Mem *result)
+{
+ if (is_result_null(left, right, result, FIELD_TYPE_INTEGER))
+   return 0;
+ int64_t a;
+ int64_t b;
+ if (bitwise_prepare(left, right, &a, &b) != 0)
+   return -1;
+ if (b <= -64)
+   result->u.i = 0;
+ else if (b < 0)
+   result->u.i = a << -b;
+ else if (b > 64)
+   result->u.i = a >= 0 ? 0 : -1;
+ else
+   result->u.i = a >> b;
+ result->flags = result->u.i < 0 ? MEM_Int : MEM_UInt;
+ return 0;
+}
+
+int
+mem_bit_not(const struct Mem *mem, struct Mem *result)
+{
+ mem_clear(result);
+ result->field_type = FIELD_TYPE_INTEGER;
+ if ((mem->flags & MEM_Null) != 0)
+   return 0;
+ int64_t i;
+ bool unused;
+ if (sqlVdbeIntValue(mem, &i, &unused) != 0) {
+   diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(mem),
+      "integer");
+   return -1;
+ }
+ result->u.i = ~i;
+ result->flags = result->u.i < 0 ? MEM_Int : MEM_UInt;
+ return 0;
+}
+
 static int
 compare_blobs(const struct Mem *a, const struct Mem *b, int *result)
 {
@@ -2065,7 +2174,7 @@ mem_value_bool(const struct Mem *mem, bool *b)
  * If pMem represents a string value, its encoding might be changed.
  */
 int
-sqlVdbeIntValue(Mem * pMem, int64_t *i, bool *is_neg)
+sqlVdbeIntValue(const struct Mem *pMem, int64_t *i, bool *is_neg)
 {
  int flags;
  assert(EIGHT_BYTE_ALIGNMENT(pMem));
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 6c022d8d8..e3b55644e 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -226,6 +226,35 @@ mem_div(const struct Mem *left, const struct Mem *right, struct Mem *result);
 int
 mem_rem(const struct Mem *left, const struct Mem *right, struct Mem *result);
 
+/** Perform a bitwise AND for two MEMs and write the result to the third MEM. */
+int
+mem_bit_and(const struct Mem *left, const struct Mem *right,
+     struct Mem *result);
+
+/** Perform a bitwise OR for two MEMs and write the result to the third MEM. */
+int
+mem_bit_or(const struct Mem *left, const struct Mem *right, struct Mem *result);
+
+/**
+ * Perform a bitwise left shift for the first MEM by value from the second MEM
+ * and write the result to the third MEM.
+ */
+int
+mem_shift_left(const struct Mem *left, const struct Mem *right,
+        struct Mem *result);
+
+/**
+ * Perform a bitwise right shift for the first MEM by value from the second MEM
+ * and write the result to the third MEM.
+ */
+int
+mem_shift_right(const struct Mem *left, const struct Mem *right,
+   struct Mem *result);
+
+/** Perform a bitwise NOT to the MEM and write the result to the second MEM. */
+int
+mem_bit_not(const struct Mem *mem, struct Mem *result);
+
 /** Compare two non-NULL MEMs and return the result of comparison. */
 int
 mem_compare(const struct Mem *left, const struct Mem *right, int *result,
@@ -464,7 +493,7 @@ releaseMemArray(Mem * p, int N);
 
 int
 mem_value_bool(const struct Mem *mem, bool *b);
-int sqlVdbeIntValue(struct Mem *, int64_t *, bool *is_neg);
+int sqlVdbeIntValue(const struct Mem *, int64_t *, bool *is_neg);
 int sqlVdbeRealValue(struct Mem *, double *);
 const void *
 sql_value_blob(struct Mem *);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 6a923a8b6..2ad681fa4 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1341,6 +1341,16 @@ case OP_FunctionByName: {
  * store the result in register P3.
  * If either input is NULL, the result is NULL.
  */
+case OP_BitAnd: {               /* same as TK_BITAND, in1, in2, out3 */
+ pIn1 = &aMem[pOp->p1];
+ pIn2 = &aMem[pOp->p2];
+ pOut = &aMem[pOp->p3];
+ if (mem_bit_and(pIn2, pIn1, pOut) != 0)
+   goto abort_due_to_error;
+ assert(pOut->field_type == FIELD_TYPE_INTEGER);
+ break;
+}
+
 /* Opcode: BitOr P1 P2 P3 * *
  * Synopsis: r[P3]=r[P1]|r[P2]
  *
@@ -1348,6 +1358,16 @@ case OP_FunctionByName: {
  * store the result in register P3.
  * If either input is NULL, the result is NULL.
  */
+case OP_BitOr: {                /* same as TK_BITOR, in1, in2, out3 */
+ pIn1 = &aMem[pOp->p1];
+ pIn2 = &aMem[pOp->p2];
+ pOut = &aMem[pOp->p3];
+ if (mem_bit_or(pIn2, pIn1, pOut) != 0)
+   goto abort_due_to_error;
+ assert(pOut->field_type == FIELD_TYPE_INTEGER);
+ break;
+}
+
 /* Opcode: ShiftLeft P1 P2 P3 * *
  * Synopsis: r[P3]=r[P2]<<r[P1]
  *
@@ -1356,6 +1376,16 @@ case OP_FunctionByName: {
  * Store the result in register P3.
  * If either input is NULL, the result is NULL.
  */
+case OP_ShiftLeft: {            /* same as TK_LSHIFT, in1, in2, out3 */
+ pIn1 = &aMem[pOp->p1];
+ pIn2 = &aMem[pOp->p2];
+ pOut = &aMem[pOp->p3];
+ if (mem_shift_left(pIn2, pIn1, pOut) != 0)
+   goto abort_due_to_error;
+ assert(pOut->field_type == FIELD_TYPE_INTEGER);
+ break;
+}
+
 /* Opcode: ShiftRight P1 P2 P3 * *
  * Synopsis: r[P3]=r[P2]>>r[P1]
  *
@@ -1364,64 +1394,13 @@ case OP_FunctionByName: {
  * Store the result in register P3.
  * If either input is NULL, the result is NULL.
  */
-case OP_BitAnd:                 /* same as TK_BITAND, in1, in2, out3 */
-case OP_BitOr:                  /* same as TK_BITOR, in1, in2, out3 */
-case OP_ShiftLeft:              /* same as TK_LSHIFT, in1, in2, out3 */
 case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
- i64 iA;
- u64 uA;
- i64 iB;
- u8 op;
-
  pIn1 = &aMem[pOp->p1];
  pIn2 = &aMem[pOp->p2];
- pOut = vdbe_prepare_null_out(p, pOp->p3);
- if (mem_is_any_null(pIn1, pIn2)) {
-   /* Force NULL be of type INTEGER. */
-   pOut->field_type = FIELD_TYPE_INTEGER;
-   break;
- }
- bool unused;
- if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) {
-   diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-      mem_str(pIn2), "integer");
-   goto abort_due_to_error;
- }
- if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) {
-   diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-      mem_str(pIn1), "integer");
+ pOut = &aMem[pOp->p3];
+ if (mem_shift_right(pIn2, pIn1, pOut) != 0)
    goto abort_due_to_error;
- }
- op = pOp->opcode;
- if (op==OP_BitAnd) {
-   iA &= iB;
- } else if (op==OP_BitOr) {
-   iA |= iB;
- } else if (iB!=0) {
-   assert(op==OP_ShiftRight || op==OP_ShiftLeft);
-
-   /* If shifting by a negative amount, shift in the other direction */
-   if (iB<0) {
-     assert(OP_ShiftRight==OP_ShiftLeft+1);
-     op = 2*OP_ShiftLeft + 1 - op;
-     iB = iB>(-64) ? -iB : 64;
-   }
-
-   if (iB>=64) {
-     iA = (iA>=0 || op==OP_ShiftLeft) ? 0 : -1;
-   } else {
-     memcpy(&uA, &iA, sizeof(uA));
-     if (op==OP_ShiftLeft) {
-       uA <<= iB;
-     } else {
-       uA >>= iB;
-       /* Sign-extend on a right shift of a negative number */
-       if (iA<0) uA |= ((((u64)0xffffffff)<<32)|0xffffffff) << (64-iB);
-     }
-     memcpy(&iA, &uA, sizeof(iA));
-   }
- }
- mem_set_i64(pOut, iA);
+ assert(pOut->field_type == FIELD_TYPE_INTEGER);
  break;
 }
 
@@ -1903,19 +1882,9 @@ case OP_Not: {                /* same as TK_NOT, in1, out2 */
  */
 case OP_BitNot: {             /* same as TK_BITNOT, in1, out2 */
  pIn1 = &aMem[pOp->p1];
- pOut = vdbe_prepare_null_out(p, pOp->p2);
- /* Force NULL be of type INTEGER. */
- pOut->field_type = FIELD_TYPE_INTEGER;
- if (!mem_is_null(pIn1)) {
-   int64_t i;
-   bool is_neg;
-   if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) {
-     diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-        mem_str(pIn1), "integer");
-     goto abort_due_to_error;
-   }
-   mem_set_i64(pOut, ~i);
- }
+ pOut = &aMem[pOp->p2];
+ if (mem_bit_not(pIn1, pOut) != 0)
+   goto abort_due_to_error;
  break;
 }