[tarantool-patches] Re: [PATCH 1/2] sql: extend struct Mem with field_type member

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Fri Aug 2 02:11:21 MSK 2019


>>> diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
>>> index 489a5a91c..8a509da96 100644
>>> --- a/test/sql/types.test.lua
>>> +++ b/test/sql/types.test.lua
>>> @@ -407,6 +407,12 @@ box.execute("CREATE TABLE t (id INT PRIMARY KEY, a INT, s SCALAR);")
>>> box.execute("INSERT INTO t VALUES (1, 1, 1), (2, NULL, NULL);")
>>> box.execute("SELECT typeof(a), typeof(s) FROM t;")
>>>
>>> +box.execute('CREATE TABLE t1 (id INTEGER PRIMARY KEY, a INTEGER, b INTEGER)')
>>> +box.execute('INSERT INTO t1 VALUES (1, NULL, NULL);')
>>> +box.execute('SELECT typeof(a & b) FROM t1;')
>>> +box.execute('SELECT typeof(a), typeof(b), typeof(a & b) FROM t1')
>>> +
>>> box.execute("SELECT typeof(CAST(0 AS UNSIGNED));")
>>>
>>> box.space.T:drop()
>>> +box.space.T1:drop()
>>>
>>>> The same about all other places, where you call mem_set_* and don't
>>>> update field_type.
>>>
>>> It’s okay, since updating current value in memory cell doesn’t
>>> imply that field type should be updated as well. In most scenarios
>>> before setting new value to memory cell, we provide clean-up
>>> via memAboutToChange(), which in turn invalidates field type.
>>
>> 3. memAboutToChange is no-op in Release build, so it does not invalidate
>> anything in that mode. It means, that field_type now can be a garbage
>> from a previous value, if the memory cell was reused to assign something
>> new.
> 
> Oops, I moved field type resetting to out2prerelease() and added
> call of this function to several places.

Another missed place: vdbe.c:2110.
Also if on line vdbe.c:2462 the condition is false,
then pOut field_type can be a garbage from a previous
value. What is strange, for OP_BitNot you set field_type INTEGER
unconditionally, but for OP_Not you set it to MAX instead of BOOLEAN,
and conditionally. One of these places is definitely a bug.

In OP_MakeRecord pOut also does not set field_type, even to MAX.
There is sqlVdbeMemRelease, but it is noop if the mem didn't store
a dynamically allocated value.

Additionally, you put field_type invalidation in out2prerelease to
an unreachable path, after all returns.

Finally, OP_Fetch does not set field type even when a field is got
from a space tuple - it is used in checks:

box.cfg{}
box.execute("CREATE TABLE test (id INTEGER PRIMARY KEY, a INTEGER, CHECK(typeof(a) == 'integer'))")
box.execute("INSERT INTO test VALUES (1, 1)")
box.execute("INSERT INTO test VALUES (2, NULL)")
---
- error: 'Check constraint failed ''CK_CONSTRAINT_1_TEST'': typeof(a) == ''integer'''
...


There are still more bugs, but since 2.2 is coming, I've finished the
patch.

>>>
>>> What is more, this is the only path (OP_Cast) which calls
>>> sqlVdbeMemCast() function. So IMHO it is pretty OK.
>>>
>>> If you want, I can attempt at refactoring sqlVdbeMemCast()
>>> so that make it feature only one exit point and move filed_type
>>> assignment to it.
>>>
>>
>> 5. Never mind. Too many changes.
> 
> I’m not sure if it is a sarcasm or not...
> 

It is not. There are really too many changes, and in addition now I
think, that it is a good idea to keep all field_type updates in vdbe.c
since it can be set to a column only. And only vdbe.c fetches columns.
I don't remember using sarcasm in my reviews btw (except sometimes in
titles).

My review fixes, on top of the branch. If you apply them, then LGTM.
Otherwise give me your comments, and we will continue the work (it
won't be LGTM then, *Kirill*, don't push!).

========================================================================

commit 6a01ec5219cdc8465e37713c6092b56440421258
Author: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
Date:   Thu Aug 1 23:52:07 2019 +0200

    Review fixes

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 77529d346..143278a67 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -532,33 +532,16 @@ registerTrace(int iReg, Mem *p) {
 
 #endif
 
-/*
- * Return the register of pOp->p2 after first preparing it to be
- * overwritten with an integer value.
- */
-static SQL_NOINLINE Mem *
-out2PrereleaseWithClear(Mem *pOut)
-{
-	sqlVdbeMemSetNull(pOut);
-	pOut->flags = MEM_Int;
-	return pOut;
-}
-
-static Mem *
-out2Prerelease(Vdbe *p, VdbeOp *pOp)
+static struct Mem *
+vdbe_prepare_null_out(struct Vdbe *v, int n)
 {
-	Mem *pOut;
-	assert(pOp->p2>0);
-	assert(pOp->p2<=(p->nMem+1 - p->nCursor));
-	pOut = &p->aMem[pOp->p2];
-	memAboutToChange(p, pOut);
-	if (VdbeMemDynamic(pOut)) { /*OPTIMIZATION-IF-FALSE*/
-		return out2PrereleaseWithClear(pOut);
-	} else {
-		pOut->flags = MEM_Int;
-		return pOut;
-	}
-	pOut->field_type = field_type_MAX;
+	assert(n > 0);
+	assert(n <= (v->nMem + 1 - v->nCursor));
+	struct Mem *out = &v->aMem[n];
+	memAboutToChange(v, out);
+	sqlVdbeMemSetNull(out);
+	out->field_type = field_type_MAX;
+	return out;
 }
 
 struct stailq *
@@ -607,6 +590,17 @@ mem_type_to_str(const struct Mem *p)
 	}
 }
 
+static inline const struct tuple_field *
+vdbe_field_ref_fetch_field(struct vdbe_field_ref *field_ref, uint32_t fieldno)
+{
+	if (field_ref->tuple == NULL)
+		return NULL;
+	struct tuple_format *format = tuple_format(field_ref->tuple);
+	if (fieldno >= tuple_format_field_count(format))
+		return NULL;
+	return tuple_format_field(format, fieldno);
+}
+
 /**
  * Try to get a current tuple field using its field map.
  * @param field_ref The vdbe_field_ref instance to use.
@@ -620,12 +614,9 @@ static const void *
 vdbe_field_ref_fast_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno,
 			  uint32_t *field_size)
 {
-	if (field_ref->tuple == NULL)
-		return NULL;
-	struct tuple_format *format = tuple_format(field_ref->tuple);
-	if (fieldno >= tuple_format_field_count(format) ||
-	    tuple_format_field(format, fieldno)->offset_slot ==
-	    TUPLE_OFFSET_SLOT_NIL)
+	const struct tuple_field *tf =
+		vdbe_field_ref_fetch_field(field_ref, fieldno);
+	if (tf == NULL || tf->offset_slot == TUPLE_OFFSET_SLOT_NIL)
 		return NULL;
 	const char *field = tuple_field(field_ref->tuple, fieldno);
 	const char *end = field;
@@ -634,6 +625,16 @@ vdbe_field_ref_fast_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno,
 	return field;
 }
 
+static inline enum field_type
+vdbe_field_ref_fetch_type(struct vdbe_field_ref *field_ref, uint32_t fieldno)
+{
+	const struct tuple_field *tf =
+		vdbe_field_ref_fetch_field(field_ref, fieldno);
+	if (tf == NULL || tf->type == FIELD_TYPE_ANY)
+		return field_type_MAX;
+	return tf->type;
+}
+
 /**
  * Fetch field by fieldno using vdbe_field_ref and store result
  * in dest_mem.
@@ -647,7 +648,6 @@ static int
 vdbe_field_ref_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno,
 		     struct Mem *dest_mem)
 {
-	sqlVdbeMemSetNull(dest_mem);
 	uint32_t *slots = field_ref->slots;
 	if (fieldno >= field_ref->field_count) {
 		UPDATE_MAX_BLOBSIZE(dest_mem);
@@ -719,6 +719,7 @@ vdbe_field_ref_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno,
 		dest_mem->flags |= MEM_Term;
 	}
 	UPDATE_MAX_BLOBSIZE(dest_mem);
+	dest_mem->field_type = vdbe_field_ref_fetch_type(field_ref, fieldno);
 	return 0;
 }
 
@@ -1076,13 +1077,8 @@ case OP_Halt: {
  * The 32-bit integer value P1 is written into register P2.
  */
 case OP_Integer: {         /* out2 */
-	pOut = out2Prerelease(p, pOp);
-	if (pOp->p1 < 0) {
-		pOut->u.i = pOp->p1;
-		assert((pOut->flags & MEM_Int) != 0);
-	} else {
-		mem_set_u64(pOut, pOp->p1);
-	}
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
+	mem_set_i64(pOut, pOp->p1);
 	break;
 }
 
@@ -1092,7 +1088,7 @@ case OP_Integer: {         /* out2 */
  * The boolean value P1 is written into register P2.
  */
 case OP_Bool: {         /* out2 */
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	assert(pOp->p1 == 1 || pOp->p1 == 0);
 	mem_set_bool(pOut, pOp->p1);
 	break;
@@ -1105,7 +1101,7 @@ case OP_Bool: {         /* out2 */
  * Write that value into register P2.
  */
 case OP_Int64: {           /* out2 */
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	assert(pOp->p4.pI64!=0);
 	mem_set_int(pOut, *pOp->p4.pI64, pOp->p4type == P4_INT64);
 	break;
@@ -1118,7 +1114,7 @@ case OP_Int64: {           /* out2 */
  * Write that value into register P2.
  */
 case OP_Real: {            /* same as TK_FLOAT, out2 */
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	pOut->flags = MEM_Real;
 	assert(!sqlIsNaN(*pOp->p4.pReal));
 	pOut->u.r = *pOp->p4.pReal;
@@ -1135,7 +1131,6 @@ case OP_Real: {            /* same as TK_FLOAT, out2 */
  */
 case OP_String8: {         /* same as TK_STRING, out2 */
 	assert(pOp->p4.z!=0);
-	pOut = out2Prerelease(p, pOp);
 	pOp->opcode = OP_String;
 	pOp->p1 = sqlStrlen30(pOp->p4.z);
 
@@ -1161,7 +1156,7 @@ case OP_String8: {         /* same as TK_STRING, out2 */
  */
 case OP_String: {          /* out2 */
 	assert(pOp->p4.z!=0);
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	pOut->flags = MEM_Str|MEM_Static|MEM_Term;
 	pOut->z = pOp->p4.z;
 	pOut->n = pOp->p1;
@@ -1190,7 +1185,7 @@ case OP_NextAutoincValue: {
 	if (sequence == NULL || sequence_next(sequence, &value) != 0)
 		goto abort_due_to_error;
 
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	mem_set_i64(pOut, value);
 
 	break;
@@ -1211,7 +1206,7 @@ case OP_NextAutoincValue: {
 case OP_Null: {           /* out2 */
 	int cnt;
 	u16 nullFlag;
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	cnt = pOp->p3-pOp->p2;
 	assert(pOp->p3<=(p->nMem+1 - p->nCursor));
 	pOut->flags = nullFlag = pOp->p1 ? (MEM_Null|MEM_Cleared) : MEM_Null;
@@ -1221,6 +1216,7 @@ case OP_Null: {           /* out2 */
 		memAboutToChange(p, pOut);
 		sqlVdbeMemSetNull(pOut);
 		pOut->flags = nullFlag;
+		pOut->field_type = field_type_MAX;
 		pOut->n = 0;
 		cnt--;
 	}
@@ -1235,7 +1231,7 @@ case OP_Null: {           /* out2 */
  */
 case OP_Blob: {                /* out2 */
 	assert(pOp->p1 <= SQL_MAX_LENGTH);
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	sqlVdbeMemSetStr(pOut, pOp->p4.z, pOp->p1, 0, 0);
 	if (pOp->p3!=0) {
 		pOut->flags |= MEM_Subtype;
@@ -1262,7 +1258,7 @@ case OP_Variable: {            /* out2 */
 	if (sqlVdbeMemTooBig(pVar)) {
 		goto too_big;
 	}
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	sqlVdbeMemShallowCopy(pOut, pVar, MEM_Static);
 	UPDATE_MAX_BLOBSIZE(pOut);
 	break;
@@ -1457,10 +1453,9 @@ case OP_Concat: {           /* same as TK_CONCAT, in1, in2, out3 */
 
 	pIn1 = &aMem[pOp->p1];
 	pIn2 = &aMem[pOp->p2];
-	pOut = &aMem[pOp->p3];
+	pOut = vdbe_prepare_null_out(p, pOp->p3);
 	assert(pIn1!=pOut);
 	if ((pIn1->flags | pIn2->flags) & MEM_Null) {
-		sqlVdbeMemSetNull(pOut);
 		/* Force NULL be of type STRING. */
 		pOut->field_type = FIELD_TYPE_STRING;
 		break;
@@ -1507,7 +1502,6 @@ case OP_Concat: {           /* same as TK_CONCAT, in1, in2, out3 */
 	pOut->z[nByte+1] = 0;
 	pOut->flags |= MEM_Term;
 	pOut->n = (int)nByte;
-	pOut->field_type = field_type_MAX;
 	UPDATE_MAX_BLOBSIZE(pOut);
 	break;
 }
@@ -1568,7 +1562,7 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 	type1 = numericType(pIn1);
 	pIn2 = &aMem[pOp->p2];
 	type2 = numericType(pIn2);
-	pOut = &aMem[pOp->p3];
+	pOut = vdbe_prepare_null_out(p, pOp->p3);
 	flags = pIn1->flags | pIn2->flags;
 	if ((flags & MEM_Null)!=0) goto arithmetic_result_is_null;
 	if ((type1 & (MEM_Int | MEM_UInt)) != 0 &&
@@ -1617,7 +1611,6 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		}
 		}
 		mem_set_int(pOut, iB, is_res_neg);
-		pOut->field_type = field_type_MAX;
 	} else {
 		bIntint = 0;
 		if (sqlVdbeRealValue(pIn1, &rA) != 0) {
@@ -1655,7 +1648,6 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		}
 		pOut->u.r = rB;
 		MemSetTypeFlag(pOut, MEM_Real);
-		pOut->field_type = field_type_MAX;
 		if (((type1|type2)&MEM_Real)==0 && !bIntint) {
 			mem_apply_integer_type(pOut);
 		}
@@ -1663,7 +1655,6 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 	break;
 
 arithmetic_result_is_null:
-	sqlVdbeMemSetNull(pOut);
 	/* Force NULL be of type NUMBER. */
 	pOut->field_type = FIELD_TYPE_NUMBER;
 	break;
@@ -1768,21 +1759,18 @@ case OP_Function: {
 	 * checks to see if the register array has changed, and if so it
 	 * reinitializes the relavant parts of the sql_context object
 	 */
-	pOut = &aMem[pOp->p3];
+	pOut = vdbe_prepare_null_out(p, pOp->p3);
 	if (pCtx->pOut != pOut) {
 		pCtx->pOut = pOut;
 		for(i=pCtx->argc-1; i>=0; i--) pCtx->argv[i] = &aMem[pOp->p2+i];
 	}
 
-	memAboutToChange(p, pCtx->pOut);
 #ifdef SQL_DEBUG
 	for(i=0; i<pCtx->argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
 		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
 #endif
-	MemSetTypeFlag(pCtx->pOut, MEM_Null);
-	pCtx->pOut->field_type = field_type_MAX;
 	pCtx->is_aborted = false;
 	(*pCtx->pFunc->xSFunc)(pCtx, pCtx->argc, pCtx->argv);/* IMP: R-24505-23230 */
 
@@ -1841,9 +1829,8 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 
 	pIn1 = &aMem[pOp->p1];
 	pIn2 = &aMem[pOp->p2];
-	pOut = &aMem[pOp->p3];
+	pOut = vdbe_prepare_null_out(p, pOp->p3);
 	if ((pIn1->flags | pIn2->flags) & MEM_Null) {
-		sqlVdbeMemSetNull(pOut);
 		/* Force NULL be of type INTEGER. */
 		pOut->field_type = FIELD_TYPE_INTEGER;
 		break;
@@ -1889,7 +1876,6 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 		}
 	}
 	mem_set_i64(pOut, iA);
-	pOut->field_type = field_type_MAX;
 	break;
 }
 
@@ -2107,10 +2093,8 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 			 * The jump is taken if the SQL_JUMPIFNULL bit is set.
 			 */
 			if (pOp->p5 & SQL_STOREP2) {
-				pOut = &aMem[pOp->p2];
+				pOut = vdbe_prepare_null_out(p, pOp->p2);
 				iCompare = 1;    /* Operands are not equal */
-				memAboutToChange(p, pOut);
-				MemSetTypeFlag(pOut, MEM_Null);
 				REGISTER_TRACE(p, pOp->p2, pOut);
 			} else {
 				VdbeBranchTaken(2,3);
@@ -2228,8 +2212,6 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 	pIn3->field_type = ft_p3;
 
 	if (pOp->p5 & SQL_STOREP2) {
-		pOut = &aMem[pOp->p2];
-		pOut->field_type = field_type_MAX;
 		iCompare = res;
 		res2 = res2!=0;  /* For this path res2 must be exactly 0 or 1 */
 		if ((pOp->p5 & SQL_KEEPNULL)!=0) {
@@ -2249,7 +2231,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 			testcase( res2==1 && pOp->opcode==OP_Ne);
 			if ((pOp->opcode==OP_Eq)==res2) break;
 		}
-		memAboutToChange(p, pOut);
+		pOut = vdbe_prepare_null_out(p, pOp->p2);
 		mem_set_bool(pOut, res2);
 		REGISTER_TRACE(p, pOp->p2, pOut);
 	} else {
@@ -2438,13 +2420,9 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 		static const unsigned char or_logic[] = { 0, 1, 2, 1, 1, 1, 2, 1, 2 };
 		v1 = or_logic[v1*3+v2];
 	}
-	pOut = &aMem[pOp->p3];
-	if (v1==2) {
-		MemSetTypeFlag(pOut, MEM_Null);
-	} else {
+	pOut = vdbe_prepare_null_out(p, pOp->p3);
+	if (v1 != 2)
 		mem_set_bool(pOut, v1);
-	}
-	pOut->field_type = field_type_MAX;
 	break;
 }
 
@@ -2457,8 +2435,8 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
  */
 case OP_Not: {                /* same as TK_NOT, in1, out2 */
 	pIn1 = &aMem[pOp->p1];
-	pOut = &aMem[pOp->p2];
-	sqlVdbeMemSetNull(pOut);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
+	pOut->field_type = FIELD_TYPE_BOOLEAN;
 	if ((pIn1->flags & MEM_Null)==0) {
 		if ((pIn1->flags & MEM_Bool) == 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
@@ -2466,7 +2444,6 @@ case OP_Not: {                /* same as TK_NOT, in1, out2 */
 			goto abort_due_to_error;
 		}
 		mem_set_bool(pOut, ! pIn1->u.b);
-		pOut->field_type = field_type_MAX;
 	}
 	break;
 }
@@ -2480,8 +2457,7 @@ case OP_Not: {                /* same as TK_NOT, in1, out2 */
  */
 case OP_BitNot: {             /* same as TK_BITNOT, in1, out2 */
 	pIn1 = &aMem[pOp->p1];
-	pOut = &aMem[pOp->p2];
-	sqlVdbeMemSetNull(pOut);
+	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) {
@@ -2609,8 +2585,7 @@ case OP_Column: {
 	p2 = pOp->p2;
 
 	assert(pOp->p3>0 && pOp->p3<=(p->nMem+1 - p->nCursor));
-	pDest = &aMem[pOp->p3];
-	memAboutToChange(p, pDest);
+	pDest = vdbe_prepare_null_out(p, pOp->p3);
 	assert(pOp->p1>=0 && pOp->p1<p->nCursor);
 	assert(pC!=0);
 	assert(p2<pC->nField);
@@ -2627,8 +2602,6 @@ case OP_Column: {
 				vdbe_field_ref_prepare_data(&pC->field_ref,
 							    pReg->z, pReg->n);
 			} else {
-				sqlVdbeMemSetNull(pDest);
-				pDest->field_type = field_type_MAX;
 				goto op_column_out;
 			}
 		} else {
@@ -2699,8 +2672,7 @@ case OP_Fetch: {
 	struct vdbe_field_ref *field_ref =
 		(struct vdbe_field_ref *) p->aMem[pOp->p1].u.p;
 	uint32_t field_idx = pOp->p2;
-	struct Mem *dest_mem = &aMem[pOp->p3];
-	memAboutToChange(p, dest_mem);
+	struct Mem *dest_mem = vdbe_prepare_null_out(p, pOp->p3);
 	if (vdbe_field_ref_fetch(field_ref, field_idx, dest_mem) != 0)
 		goto abort_due_to_error;
 	REGISTER_TRACE(p, pOp->p3, dest_mem);
@@ -2784,8 +2756,7 @@ case OP_MakeRecord: {
 
 	/* Identify the output register */
 	assert(pOp->p3<pOp->p1 || pOp->p3>=pOp->p1+pOp->p2);
-	pOut = &aMem[pOp->p3];
-	memAboutToChange(p, pOut);
+	pOut = vdbe_prepare_null_out(p, pOp->p3);
 
 	/* Apply the requested types to all inputs */
 	assert(pData0<=pLast);
@@ -2860,7 +2831,7 @@ case OP_Count: {         /* out2 */
 		assert((pCrsr->curFlags & BTCF_TEphemCursor) != 0);
 		nEntry = tarantoolsqlEphemeralCount(pCrsr);
 	}
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	mem_set_u64(pOut, nEntry);
 	break;
 }
@@ -3674,7 +3645,7 @@ case OP_Found: {        /* jump, in3 */
 case OP_Sequence: {           /* out2 */
 	assert(pOp->p1>=0 && pOp->p1<p->nCursor);
 	assert(p->apCsr[pOp->p1]!=0);
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	int64_t seq_val = p->apCsr[pOp->p1]->seqCount++;
 	mem_set_u64(pOut, seq_val);
 	break;
@@ -3688,7 +3659,7 @@ case OP_Sequence: {           /* out2 */
  * incremented by one.
  */
 case OP_NextSequenceId: {
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	uint64_t id;
 	tarantoolSqlNextSeqId(&id);
 	id++;
@@ -3721,7 +3692,7 @@ case OP_NextIdEphemeral: {
 		diag_set(ClientError, ER_ROWID_OVERFLOW);
 		goto abort_due_to_error;
 	}
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	mem_set_u64(pOut, rowid);
 	break;
 }
@@ -3749,15 +3720,12 @@ case OP_FCopy: {     /* out2 */
 	}
 
 	if ((pOp->p3 & OPFLAG_NOOP_IF_NULL) && (pIn1->flags & MEM_Null)) {
-		pOut = &aMem[pOp->p2];
-		if (pOut->flags & MEM_Undefined) pOut->flags = MEM_Null;
-		pOut->field_type = field_type_MAX;
-		/* Flag is set and register is NULL -> do nothing  */
+		pOut = vdbe_prepare_null_out(p, pOp->p2);
 	} else {
 		assert(memIsValid(pIn1));
 		assert((pIn1->flags & (MEM_Int | MEM_UInt)) != 0);
 
-		pOut = &aMem[pOp->p2];
+		pOut = vdbe_prepare_null_out(p, pOp->p2);
 		mem_set_int(pOut, pIn1->u.i, pIn1->flags == MEM_Int);
 		pOut->field_type = pIn1->field_type;
 	}
@@ -3885,7 +3853,7 @@ case OP_SorterCompare: {
 case OP_SorterData: {
 	VdbeCursor *pC;
 
-	pOut = &aMem[pOp->p2];
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	pC = p->apCsr[pOp->p1];
 	assert(isSorter(pC));
 	if (sqlVdbeSorterRowkey(pC, pOut) != 0)
@@ -3929,8 +3897,7 @@ case OP_RowData: {
 	}
 #endif
 
-	pOut = out2Prerelease(p, pOp);
-	memAboutToChange(p, pOut);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 
 	assert(pOp->p1>=0 && pOp->p1<p->nCursor);
 	pC = p->apCsr[pOp->p1];
@@ -3957,7 +3924,6 @@ case OP_RowData: {
 	}
 	testcase( n==0);
 
-	sqlVdbeMemRelease(pOut);
 	if (sql_vdbe_mem_alloc_region(pOut, n) != 0)
 		goto abort_due_to_error;
 	sqlCursorPayload(pCrsr, 0, n, pOut->z);
@@ -4862,7 +4828,7 @@ case OP_Program: {        /* jump */
 case OP_Param: {           /* out2 */
 	VdbeFrame *pFrame;
 	Mem *pIn;
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 	pFrame = p->pFrame;
 	pIn = &pFrame->aMem[pOp->p1 + pFrame->aOp[pFrame->pc].p1];
 	sqlVdbeMemShallowCopy(pOut, pIn, MEM_Ephem);
@@ -4960,7 +4926,7 @@ case OP_IfPos: {        /* jump, in1 */
 case OP_OffsetLimit: {    /* in1, out2, in3 */
 	pIn1 = &aMem[pOp->p1];
 	pIn3 = &aMem[pOp->p3];
-	pOut = out2Prerelease(p, pOp);
+	pOut = vdbe_prepare_null_out(p, pOp->p2);
 
 	assert((pIn1->flags & MEM_UInt) != 0);
 	assert((pIn3->flags & MEM_UInt) != 0);
@@ -5225,7 +5191,7 @@ case OP_Init: {          /* jump */
  */
 case OP_IncMaxid: {
 	assert(pOp->p1 > 0);
-	pOut = &aMem[pOp->p1];
+	pOut = vdbe_prepare_null_out(p, pOp->p1);
 
 	if (tarantoolsqlIncrementMaxid(&pOut->u.u) != 0)
 		goto abort_due_to_error;
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index 98f906cf4..19b56d13f 100755
--- a/test/sql-tap/check.test.lua
+++ b/test/sql-tap/check.test.lua
@@ -250,13 +250,14 @@ test:do_catchsql_test(
         -- </check-2.4>
     })
 
-test:do_catchsql_test(
+test:do_execsql_test(
     "check-2.5",
     [[
         INSERT INTO t2 VALUES(4, NULL, 5, NULL);
+        DELETE FROM t2 WHERE id = 4;
     ]], {
         -- <check-2.5>
-        1, "Check constraint failed 'TWO': typeof(coalesce(y,0.1))=='number'"
+
         -- </check-2.5>
     })
 




More information about the Tarantool-patches mailing list