[tarantool-patches] [PATCH 1/5] sql: always erase numeric flag after stringifying

[Nikita Pettik]

Function which converts values to string representation
(sqlVdbeMemStringify()) erase MEM_Int/MEM_Real/MEM_Bool flags only when
it is specified by 'force' parameter. Hence, when 'force' argument is
false, memory cell after conversion will contain string value, but flag
indicating its type will be equal to combination of MEM_Str and one of
mentioned flags. It seems to be remains of affinity routines, since in
current state memory cell must have only one type.  What is more, it can
lead to unpredicted consequences, for instance assertion fault
(sql_value_type() assumes that value has one specific type). Let's fix
it removing 'force' argument from sqlVdbeMemStringify() and always clean
up type flag.
---
 src/box/sql/vdbe.c    | 8 ++++----
 src/box/sql/vdbeInt.h | 2 +-
 src/box/sql/vdbemem.c | 9 +++++----
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index c8887f9b7..9f3879910 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -183,7 +183,7 @@ vdbeTakeBranch(int iSrcLine, u8 I, u8 M)
  * already. Return non-zero if a malloc() fails.
  */
 #define Stringify(P)						\
-	if(((P)->flags&(MEM_Str|MEM_Blob))==0 && sqlVdbeMemStringify(P,0)) \
+	if(((P)->flags&(MEM_Str|MEM_Blob))==0 && sqlVdbeMemStringify(P)) \
 	{ goto no_mem; }
 
 /*
@@ -333,7 +333,7 @@ mem_apply_type(struct Mem *record, enum field_type type)
 		 */
 		if ((record->flags & MEM_Str) == 0) {
 			if ((record->flags & (MEM_Real | MEM_Int)))
-				sqlVdbeMemStringify(record, 1);
+				sqlVdbeMemStringify(record);
 		}
 		record->flags &= ~(MEM_Real | MEM_Int);
 		return 0;
@@ -2144,7 +2144,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 			if ((flags1 & MEM_Str)==0 && (flags1 & (MEM_Int|MEM_Real))!=0) {
 				testcase( pIn1->flags & MEM_Int);
 				testcase( pIn1->flags & MEM_Real);
-				sqlVdbeMemStringify(pIn1, 1);
+				sqlVdbeMemStringify(pIn1);
 				testcase( (flags1&MEM_Dyn) != (pIn1->flags&MEM_Dyn));
 				flags1 = (pIn1->flags & ~MEM_TypeMask) | (flags1 & MEM_TypeMask);
 				assert(pIn1!=pIn3);
@@ -2152,7 +2152,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 			if ((flags3 & MEM_Str)==0 && (flags3 & (MEM_Int|MEM_Real))!=0) {
 				testcase( pIn3->flags & MEM_Int);
 				testcase( pIn3->flags & MEM_Real);
-				sqlVdbeMemStringify(pIn3, 1);
+				sqlVdbeMemStringify(pIn3);
 				testcase( (flags3&MEM_Dyn) != (pIn3->flags&MEM_Dyn));
 				flags3 = (pIn3->flags & ~MEM_TypeMask) | (flags3 & MEM_TypeMask);
 			}
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 6bfeecc85..7d2470b07 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -469,7 +469,7 @@ void sqlVdbeMemInit(Mem *, sql *, u32);
 void sqlVdbeMemSetNull(Mem *);
 void sqlVdbeMemSetZeroBlob(Mem *, int);
 int sqlVdbeMemMakeWriteable(Mem *);
-int sqlVdbeMemStringify(Mem *, u8);
+int sqlVdbeMemStringify(Mem *);
 int sqlVdbeIntValue(Mem *, int64_t *);
 int sqlVdbeMemIntegerify(Mem *, bool is_forced);
 int sqlVdbeRealValue(Mem *, double *);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 4e4bd597d..d85148bc3 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -275,7 +275,7 @@ sqlVdbeMemNulTerminate(Mem * pMem)
  * user and the latter is an internal programming error.
  */
 int
-sqlVdbeMemStringify(Mem * pMem, u8 bForce)
+sqlVdbeMemStringify(Mem * pMem)
 {
 	int fg = pMem->flags;
 	const int nByte = 32;
@@ -292,16 +292,17 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce)
 	}
 	if (fg & MEM_Int) {
 		sql_snprintf(nByte, pMem->z, "%lld", pMem->u.i);
+		pMem->flags &= ~MEM_Int;
 	} else if ((fg & MEM_Bool) != 0) {
 		sql_snprintf(nByte, pMem->z, "%s", pMem->u.b ? "true" : "false");
+		pMem->flags &= ~MEM_Bool;
 	} else {
 		assert(fg & MEM_Real);
 		sql_snprintf(nByte, pMem->z, "%!.15g", pMem->u.r);
+		pMem->flags &= ~MEM_Real;
 	}
 	pMem->n = sqlStrlen30(pMem->z);
 	pMem->flags |= MEM_Str | MEM_Term;
-	if (bForce)
-		pMem->flags &= ~(MEM_Int | MEM_Real);
 	return 0;
 }
 
@@ -1107,7 +1108,7 @@ valueToText(sql_value * pVal)
 		pVal->flags |= MEM_Str;
 		sqlVdbeMemNulTerminate(pVal);	/* IMP: R-31275-44060 */
 	} else {
-		sqlVdbeMemStringify(pVal, 0);
+		sqlVdbeMemStringify(pVal);
 		assert(0 == (1 & SQL_PTR_TO_INT(pVal->z)));
 	}
 	return pVal->z;
-- 
2.15.1