Re: [Tarantool-patches] [PATCH v1 1/1] sql: introduce DECIMAL to SQL built-in functions

[Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thank you the review! My answer and diff below.

On Mon, Nov 15, 2021 at 10:39:38PM +0100, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> > diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> > index 5abaf490d..684fcc275 100644
> > --- a/src/box/sql/func.c
> > +++ b/src/box/sql/func.c
> > @@ -258,6 +258,20 @@ func_abs_double(struct sql_context *ctx, int argc, const struct Mem *argv)
> >  	mem_set_double(ctx->pOut, arg->u.r < 0 ? -arg->u.r : arg->u.r);
> >  }
> >  
> > +static void
> > +func_abs_dec(struct sql_context *ctx, int argc, const struct Mem *argv)
> > +{
> > +	assert(argc == 1);
> > +	(void)argc;
> > +	const struct Mem *arg = &argv[0];
> > +	if (mem_is_null(arg))
> > +		return;
> > +	assert(mem_is_dec(arg));
> > +	decimal_t dec;
> > +	decimal_abs(&dec, &arg->u.d);
> > +	mem_set_dec(ctx->pOut, &dec);
> 
> You should be able to use the same argument both for in and out
> values:
> 
> 	decimal_abs(&arg->u.d, &arg->u.d);
> 
> In the 'documentation' of decNumber it is said to be allowed.
> See the comment on the function 'decNumberAbs()'.
Fixed. Although we still need to copy from arg to pOut.

Diff:

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 684fcc275..e3ce6c18e 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -267,9 +267,8 @@ func_abs_dec(struct sql_context *ctx, int argc, const struct Mem *argv)
 	if (mem_is_null(arg))
 		return;
 	assert(mem_is_dec(arg));
-	decimal_t dec;
-	decimal_abs(&dec, &arg->u.d);
-	mem_set_dec(ctx->pOut, &dec);
+	mem_set_dec(ctx->pOut, &arg->u.d);
+	decimal_abs(&ctx->pOut->u.d, &ctx->pOut->u.d);
 }
 
 /** Implementation of the CHAR_LENGTH() function. */