[Tarantool-patches] [PATCH 2/6] sql: remove mem_apply_type() from OP_MakeRecord
Mergen Imeev
imeevma at tarantool.org
Tue Jun 9 14:43:58 MSK 2020
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 <imeevma at gmail.com>
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 at 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->p3<pOp->p1 || 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, {
-- <in3-3.5>
- 1, true
+ 1, false
-- </in3-3.5>
})
@@ -378,7 +378,7 @@ test:do_test(
return exec_neph(" SELECT y IN (SELECT c FROM t1) FROM t2 ")
end, {
-- <in3-3.7>
- 1, true
+ 1, false
-- </in3-3.7>
})
More information about the Tarantool-patches
mailing list