From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp57.i.mail.ru (smtp57.i.mail.ru [217.69.128.37]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 854D4469710 for ; Tue, 9 Jun 2020 14:43:59 +0300 (MSK) References: <92fc506a755863ccdf807b232dca5b96737708c6.1590671266.git.imeevma@gmail.com> From: Mergen Imeev Message-ID: Date: Tue, 9 Jun 2020 14:43:58 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH 2/6] sql: remove mem_apply_type() from OP_MakeRecord List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , tsafin@tarantool.org, tarantool-patches@dev.tarantool.org Thank you for the review! New patch below. On 01.06.2020 20:03, Vladislav Shpilevoy wrote: > Thanks for the patch! > >> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c >> index 2a941025c..fc41ee0d6 100644 >> --- a/src/box/sql/vdbe.c >> +++ b/src/box/sql/vdbe.c >> @@ -2902,7 +2902,6 @@ case OP_CheckType: { >> * into ephemeral space. Thus, sort of memory optimization can be performed. >> */ > P4 should be removed from the comment too. Fixed. > >> case OP_MakeRecord: { >> - Mem *pRec; /* The new record */ >> Mem *pData0; /* First field to be combined into the record */ >> Mem MAYBE_UNUSED *pLast; /* Last field of the record */ >> int nField; /* Number of fields in the record */ New patch: From f87e9b757844bde856bc41de5ad90754c66b0ef3 Mon Sep 17 00:00:00 2001 From: Mergen Imeev Date: Wed, 27 May 2020 11:16:43 +0300 Subject: [PATCH] sql: remove mem_apply_type() from OP_MakeRecord This patch removes type changing from OP_MakeRecord. Part of #4230 diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index f74f9b358..23fbb15de 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -965,12 +965,7 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space)          sqlVdbeAddOp2(v, OP_Next, idx_cursor, next_row_addr);          /* Add the entry to the stat1 table. */          callStatGet(v, stat4_reg, STAT_GET_STAT1, stat1_reg); -        enum field_type types[4] = { FIELD_TYPE_STRING, -                         FIELD_TYPE_STRING, -                         FIELD_TYPE_STRING, -                         field_type_MAX }; -        sqlVdbeAddOp4(v, OP_MakeRecord, tab_name_reg, 4, tmp_reg, -                  (char *)types, sizeof(types)); +        sqlVdbeAddOp3(v, OP_MakeRecord, tab_name_reg, 4, tmp_reg);          sqlVdbeAddOp4(v, OP_IdxInsert, tmp_reg, 0, 0,                    (char *)stat1, P4_SPACEPTR);          /* Add the entries to the stat4 table. */ diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index 68abd1f58..0b6f0bd62 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -328,12 +328,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,               * key.               */              key_len = 0; -            struct index *pk = space_index(space, 0); -            enum field_type *types = is_view ? NULL : -                         sql_index_type_str(parse->db, -                                    pk->def); -            sqlVdbeAddOp4(v, OP_MakeRecord, reg_pk, pk_len, -                      reg_key, (char *)types, P4_DYNAMIC); +            sqlVdbeAddOp3(v, OP_MakeRecord, reg_pk, pk_len, +                      reg_key);              /* Set flag to save memory allocating one               * by malloc.               */ diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index bc2182446..019628a26 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2859,8 +2859,6 @@ sqlCodeSubselect(Parse * pParse,    /* Parsing context */                  struct ExprList_item *pItem;                  int r1, r2, r3; -                enum field_type lhs_type = -                    sql_expr_type(pLeft);                  bool unused;                  struct coll *unused_coll;                  if (sql_expr_coll(pParse, pExpr->pLeft, &unused, @@ -2886,11 +2884,7 @@ sqlCodeSubselect(Parse * pParse,    /* Parsing context */                          jmpIfDynamic = -1;                      }                      r3 = sqlExprCodeTarget(pParse, pE2, r1); -                    enum field_type types[2] = -                        { lhs_type, field_type_MAX }; -                     sqlVdbeAddOp4(v, OP_MakeRecord, r3, -                              1, r2, (char *)types, -                              sizeof(types)); +                    sqlVdbeAddOp3(v, OP_MakeRecord, r3, 1, r2);                      sql_expr_type_cache_change(pParse,                                     r3, 1);                      sqlVdbeAddOp2(v, OP_IdxInsert, r2, diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c index 482220a95..43471d51a 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -262,13 +262,8 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent,                        link->child_field + 1 + reg_data,                        temp_regs + i);          } -        struct index *idx = space_index(parent, referenced_idx); -        assert(idx != NULL); -        sqlVdbeAddOp4(v, OP_MakeRecord, temp_regs, field_count, -                  rec_reg, -                  (char *) sql_index_type_str(parse_context->db, -                                  idx->def), -                  P4_DYNAMIC); +        sqlVdbeAddOp3(v, OP_MakeRecord, temp_regs, field_count, +                  rec_reg);          sqlVdbeAddOp4Int(v, OP_Found, cursor, ok_label, rec_reg, 0);          sqlReleaseTempReg(parse_context, rec_reg);          sqlReleaseTempRange(parse_context, temp_regs, field_count); diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 0b7358af4..f3f5dad76 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1270,13 +1270,8 @@ selectInnerLoop(Parse * pParse,        /* The parser context */                             regOrig, nResultCol, nPrefixReg);              } else {                  int r1 = sqlGetTempReg(pParse); -                enum field_type *types = -                    field_type_sequence_dup(pParse, -                                pDest->dest_type, -                                nResultCol); -                sqlVdbeAddOp4(v, OP_MakeRecord, regResult, -                          nResultCol, r1, (char *)types, -                          P4_DYNAMIC); +                sqlVdbeAddOp3(v, OP_MakeRecord, regResult, +                          nResultCol, r1);                  sql_expr_type_cache_change(pParse,                                 regResult,                                 nResultCol); @@ -1693,12 +1688,8 @@ generateSortTail(Parse * pParse,    /* Parsing context */              break;          }      case SRT_Set:{ -            enum field_type *types = -                field_type_sequence_dup(pParse, pDest->dest_type, -                            nColumn); -            sqlVdbeAddOp4(v, OP_MakeRecord, regRow, nColumn, -                      regTupleid, (char *)types, -                      P4_DYNAMIC); +            sqlVdbeAddOp3(v, OP_MakeRecord, regRow, nColumn, +                      regTupleid);              sql_expr_type_cache_change(pParse, regRow, nColumn);              sqlVdbeAddOp2(v, OP_IdxInsert, regTupleid, pDest->reg_eph);              break; @@ -3136,12 +3127,8 @@ generateOutputSubroutine(struct Parse *parse, struct Select *p,              int r1;              testcase(in->nSdst > 1);              r1 = sqlGetTempReg(parse); -            enum field_type *types = -                field_type_sequence_dup(parse, dest->dest_type, -                            in->nSdst); -            sqlVdbeAddOp4(v, OP_MakeRecord, in->iSdst, -                      in->nSdst, r1, (char *)types, -                      P4_DYNAMIC); +            sqlVdbeAddOp3(v, OP_MakeRecord, in->iSdst, in->nSdst, +                      r1);              sql_expr_type_cache_change(parse, in->iSdst,                             in->nSdst);              sqlVdbeAddOp2(v, OP_IdxInsert, r1, dest->reg_eph); diff --git a/src/box/sql/update.c b/src/box/sql/update.c index 24c7cfa27..22f82390c 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -251,11 +251,7 @@ sqlUpdate(Parse * pParse,        /* The parser context */          nKey = pk_part_count;          regKey = iPk;      } else { -        enum field_type *types = is_view ? NULL : -                     sql_index_type_str(pParse->db, -                                pPk->def); -        sqlVdbeAddOp4(v, OP_MakeRecord, iPk, pk_part_count, -                  regKey, (char *) types, P4_DYNAMIC); +        sqlVdbeAddOp3(v, OP_MakeRecord, iPk, pk_part_count, regKey);          /*           * Set flag to save memory allocating one by           * malloc. @@ -420,12 +416,8 @@ sqlUpdate(Parse * pParse,        /* The parser context */              int key_reg;              if (okOnePass) {                  key_reg = sqlGetTempReg(pParse); -                enum field_type *types = -                    sql_index_type_str(pParse->db, -                               pPk->def); -                sqlVdbeAddOp4(v, OP_MakeRecord, iPk, -                          pk_part_count, key_reg, -                          (char *) types, P4_DYNAMIC); +                sqlVdbeAddOp3(v, OP_MakeRecord, iPk, +                          pk_part_count, key_reg);              } else {                  assert(nKey == 0);                  key_reg = regKey; diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index ae2622c9e..eb5c89e9d 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2908,24 +2908,17 @@ case OP_CheckType: {      break;  } -/* Opcode: MakeRecord P1 P2 P3 P4 P5 +/* Opcode: MakeRecord P1 P2 P3 * P5   * Synopsis: r[P3]=mkrec(r[P1@P2])   *   * Convert P2 registers beginning with P1 into the [record format]   * use as a data record in a database table or as a key   * in an index.  The OP_Column opcode can decode the record later.   * - * P4 may be a string that is P2 characters long.  The nth character of the - * string indicates the column type that should be used for the nth - * field of the index key. - * - * If P4 is NULL then all index fields have type SCALAR. - *   * If P5 is not NULL then record under construction is intended to be inserted   * into ephemeral space. Thus, sort of memory optimization can be performed.   */  case OP_MakeRecord: { -    Mem *pRec;             /* The new record */      Mem *pData0;           /* First field to be combined into the record */      Mem MAYBE_UNUSED *pLast;  /* Last field of the record */      int nField;            /* Number of fields in the record */ @@ -2947,7 +2940,6 @@ case OP_MakeRecord: {       * of the record to data0.       */      nField = pOp->p1; -    enum field_type *types = pOp->p4.types;      bIsEphemeral = pOp->p5;      assert(nField>0 && pOp->p2>0 && pOp->p2+nField<=(p->nMem+1 - p->nCursor)+1);      pData0 = &aMem[nField]; @@ -2958,15 +2950,6 @@ case OP_MakeRecord: {      assert(pOp->p3p1 || pOp->p3>=pOp->p1+pOp->p2);      pOut = vdbe_prepare_null_out(p, pOp->p3); -    /* Apply the requested types to all inputs */ -    assert(pData0<=pLast); -    if (types != NULL) { -        pRec = pData0; -        do { -            mem_apply_type(pRec++, *(types++)); -        } while(types[0] != field_type_MAX); -    } -      struct region *region = &fiber()->gc;      size_t used = region_used(region);      uint32_t tuple_size; diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua index e29db9d93..f7681640e 100755 --- a/test/sql-tap/in3.test.lua +++ b/test/sql-tap/in3.test.lua @@ -354,7 +354,7 @@ test:do_test(          return exec_neph(" SELECT y IN (SELECT a FROM t1) FROM t2 ")      end, {          -- -        1, true +        1, false          --      }) @@ -378,7 +378,7 @@ test:do_test(          return exec_neph(" SELECT y IN (SELECT c FROM t1) FROM t2 ")      end, {          -- -        1, true +        1, false          --      })