[Tarantool-patches] [PATCH v5 37/52] sql: introduce mem_set_null_clear()

[Mergen Imeev]

Thank you for the review! My answer, diff and new patch below.

On Tue, Apr 13, 2021 at 01:38:18AM +0200, Vladislav Shpilevoy wrote:
> Nice fixes!
> 
> > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> > index 7f1e0bcbe..4566606d7 100644
> > --- a/src/box/sql/vdbe.c
> > +++ b/src/box/sql/vdbe.c
> > @@ -855,7 +855,7 @@ case OP_String: {          /* out2 */
> >   * is less than P2 (typically P3 is zero) then only register P2 is
> >   * set to NULL.
> >   *
> > - * If the P1 value is non-zero, then also set the MEM_Cleared flag so that
> > + * If the P1 value is non-zero, then also set the Cleared flag so that
> 
> The flag is still here. If you didn't remove it, I would propose to
> keep its mentions in the comments.
> 
Fixed, dropped this change.

> >   * NULL values will not compare equal even if SQL_NULLEQ is set on
> >   * OP_Ne or OP_Eq.
> >   */


Diff:


diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 116194fae..e07ff1be9 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -855,7 +855,7 @@ case OP_String: {          /* out2 */
  * is less than P2 (typically P3 is zero) then only register P2 is
  * set to NULL.
  *
- * If the P1 value is non-zero, then also set the Cleared flag so that
+ * If the P1 value is non-zero, then also set the MEM_Cleared flag so that
  * NULL values will not compare equal even if SQL_NULLEQ is set on
  * OP_Ne or OP_Eq.
  */



New patch:


commit 56565414c4f332d5711d458b3896016d972c1f9c
Author: Mergen Imeev <imeevma at gmail.com>
Date:   Tue Mar 16 15:14:32 2021 +0300

    sql: introduce mem_set_null_clear()
    
    This patch introduces mem_set_null_clear() function. This function sets
    "cleared" NULL to MEM. This NULL is used only in internal VDBE
    operations.
    
    Part of #5818

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 172883a44..3930067e1 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -485,6 +485,13 @@ mem_set_agg(struct Mem *mem, struct func *func, int size)
 	return 0;
 }
 
+void
+mem_set_null_clear(struct Mem *mem)
+{
+	mem_clear(mem);
+	mem->flags = MEM_Null | MEM_Cleared;
+}
+
 int
 mem_copy(struct Mem *to, const struct Mem *from)
 {
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 6dee83dc5..9890762e0 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -553,6 +553,10 @@ mem_set_frame(struct Mem *mem, struct VdbeFrame *frame);
 int
 mem_set_agg(struct Mem *mem, struct func *func, int size);
 
+/** Clear MEM and set it to special, "cleared", NULL. */
+void
+mem_set_null_clear(struct Mem *mem);
+
 /**
  * Copy content of MEM from one MEM to another. In case source MEM contains
  * string or binary and allocation type is not STATIC, this value is copied to
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index d5b8033a4..e07ff1be9 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -865,13 +865,14 @@ case OP_Null: {           /* out2 */
 	cnt = pOp->p3-pOp->p2;
 	assert(pOp->p3<=(p->nMem+1 - p->nCursor));
 	if (pOp->p1 != 0)
-		pOut->flags = MEM_Null | MEM_Cleared;
+		mem_set_null_clear(pOut);
 	while( cnt>0) {
 		pOut++;
 		memAboutToChange(p, pOut);
-		mem_set_null(pOut);
 		if (pOp->p1 != 0)
-			pOut->flags = MEM_Null | MEM_Cleared;
+			mem_set_null_clear(pOut);
+		else
+			mem_set_null(pOut);
 		cnt--;
 	}
 	break;