[tarantool-patches] Re: [PATCH 8/9] sql: make LIKE predicate return boolean result

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Mon Apr 22 21:02:17 MSK 2019


Thanks for the fixes!

> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
> index 1590a3550..69aea5ef5 100644
> --- a/src/box/sql/vdbemem.c
> +++ b/src/box/sql/vdbemem.c
> @@ -803,7 +803,7 @@ sqlVdbeMemSetInt64(Mem * pMem, i64 val)
>  }
>  
>  void
> -vdbe_mem_set_boolean(struct Mem *mem, bool value)
> +vdbe_mem_set_bool(struct Mem *mem, bool value)
>  {
> 

1. We now have mem_value_bool(), and so as to be consistent
it would be better to name this function mem_set_bool(), not
vdbe_mem_set_bool(). It does not take struct Vdbe pointer
anyway.

2. Is there any concrete reason why vdbe_mem_set_bool() is
not used now in the following places?

vdbe.c:1101
	pOut = out2Prerelease(p, pOp);
	assert(pOp->p1 == 1 || pOp->p1 == 0);
	pOut->flags = MEM_Bool;
	pOut->u.b = pOp->p1;
	break;

vdbe.c:2275
	memAboutToChange(p, pOut);
	MemSetTypeFlag(pOut, MEM_Bool);
	pOut->u.b = res2;
	REGISTER_TRACE(pOp->p2, pOut);

vdbe.c:2471
	pOut->u.b = v1;
	MemSetTypeFlag(pOut, MEM_Bool);

vdbe.c:2495
	pOut->flags = MEM_Bool;
	pOut->u.b = ! pIn1->u.b;

If it works for them, then I propose to move vdbe_mem_set_bool()
to the patch 'sql: introduce type boolean', and use it.

Also, please, consider this minor review fix, on the branch:

==================================================================
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 8426f9b5f..07c887bb4 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -499,7 +499,7 @@ void
 sql_result_int(sql_context *, int);
 
 void
-sql_result_bool(sql_context *ctx, bool value);
+sql_result_bool(struct sql_context *ctx, bool value);
 
 void
 sql_result_int64(sql_context *, sql_int64);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index c2a319dbd..9a405c8ee 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -387,7 +387,7 @@ sql_result_int(sql_context * pCtx, int iVal)
 }
 
 void
-sql_result_bool(sql_context *ctx, bool value)
+sql_result_bool(struct sql_context *ctx, bool value)
 {
 	vdbe_mem_set_bool(ctx->pOut, value);
 }




More information about the Tarantool-patches mailing list