[Tarantool-patches] [PATCH v5 09/52] sql: introduce mem_str()

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Thu Apr 15 01:23:36 MSK 2021


Good job on the fixes!

> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
> index 79c3d60e0..907c9f5c6 100644
> --- a/src/box/sql/vdbeaux.c
> +++ b/src/box/sql/vdbeaux.c
> @@ -1108,21 +1108,10 @@ displayP4(Op * pOp, char *zTemp, int nTemp)
>  			break;
>  		}
>  	case P4_MEM:{
> -			Mem *pMem = pOp->p4.pMem;
> -			if (pMem->flags & MEM_Str) {
> -				zP4 = pMem->z;
> -			} else if (pMem->flags & MEM_Int) {
> -				sqlXPrintf(&x, "%lld", pMem->u.i);
> -			} else if (pMem->flags & MEM_UInt) {
> -				sqlXPrintf(&x, "%llu", pMem->u.u);
> -			} else if (pMem->flags & MEM_Real) {
> -				sqlXPrintf(&x, "%.16g", pMem->u.r);
> -			} else if (pMem->flags & MEM_Null) {
> -				zP4 = "NULL";
> -			} else {
> -				assert(pMem->flags & MEM_Blob);
> -				zP4 = "(binary string)";
> -			}
> +			const char *value = mem_str(pOp->p4.pMem);
> +			int len = strlen(value);
> +			uint32_t size = MIN(len, nTemp - 1);

Why do you need that limit? The accum is created as
sqlStrAccumInit(&x, 0, zTemp, nTemp, 0). The last argument 0 means
it won't realloc anything, so it is safe to append. The same in the
next diff hunk.

Consider this diff:

====================
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index a2a0fc33e..95a035fd4 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -52,7 +52,9 @@ mem_str(const struct Mem *mem)
 	case MEM_Null:
 		return "NULL";
 	case MEM_Str:
-		return tt_sprintf("%.*s", mem->n, mem->z);
+		if ((mem->flags & MEM_Term) != 0)
+			return mem->z;
+		return tt_cstr(mem->z, mem->n);
 	case MEM_Int:
 		return tt_sprintf("%lld", mem->u.i);
 	case MEM_UInt:
@@ -68,9 +70,8 @@ mem_str(const struct Mem *mem)
 	case MEM_Bool:
 		return mem->u.b ? "TRUE" : "FALSE";
 	default:
-		break;
+		return "unknown";
 	}
-	return "unknown";
 }
 
 static inline bool
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 907c9f5c6..9ef0445bd 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1109,9 +1109,7 @@ displayP4(Op * pOp, char *zTemp, int nTemp)
 		}
 	case P4_MEM:{
 			const char *value = mem_str(pOp->p4.pMem);
-			int len = strlen(value);
-			uint32_t size = MIN(len, nTemp - 1);
-			sqlStrAccumAppend(&x, value, size);
+			sqlStrAccumAppend(&x, value, strlen(value));
 			break;
 		}
 	case P4_INTARRAY:{
diff --git a/src/box/sql/vdbetrace.c b/src/box/sql/vdbetrace.c
index 677de65e2..a26880041 100644
--- a/src/box/sql/vdbetrace.c
+++ b/src/box/sql/vdbetrace.c
@@ -146,9 +146,7 @@ sqlVdbeExpandSql(Vdbe * p,	/* The prepared statement being evaluated */
 			nextIndex = idx + 1;
 			assert(idx > 0 && idx <= p->nVar);
 			const char *value = mem_str(&p->aVar[idx - 1]);
-			uint32_t len = strlen(value);
-			uint32_t size = MIN(len, sizeof(zBase) - 1);
-			sqlStrAccumAppend(&out, value, size);
+			sqlStrAccumAppend(&out, value, strlen(value));
 		}
 	}
 	if (out.accError)


More information about the Tarantool-patches mailing list