Re: [Tarantool-patches] [PATCH v2 1/3] sql: initialize MEM used in aggregate functions

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

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

On Thu, Apr 29, 2021 at 11:02:06PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> > diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> > index 9c28d5122..d282b2cea 100644
> > --- a/src/box/sql/func.c
> > +++ b/src/box/sql/func.c
> > @@ -1798,14 +1798,13 @@ minmaxStep(sql_context * context, int NotUsed, sql_value ** argv)
> >  static void
> >  minMaxFinalize(sql_context * context)
> >  {
> > -	sql_value *pRes;
> > -	pRes = (sql_value *) sql_aggregate_context(context, 0);
> > -	if (pRes) {
> > -		if (pRes->flags) {
> > -			sql_result_value(context, pRes);
> > -		}
> > -		mem_destroy(pRes);
> > -	}
> > +	struct Mem *mem = context->pMem;
> > +	struct Mem *res;
> > +	if (!mem_is_agg(mem) || mem_get_agg(mem, (void **)&res) != 0)
> 
> You don't need `is_agg` check. `get` does the same already.
> 
Fixed.

> > +		return;
> > +	if (!mem_is_null(res))
> > +		sql_result_value(context, res);
> > +	mem_destroy(res);
> >  }

Diff:


diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index d282b2cea..94302cc56 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1800,7 +1800,7 @@ minMaxFinalize(sql_context * context)
 {
 	struct Mem *mem = context->pMem;
 	struct Mem *res;
-	if (!mem_is_agg(mem) || mem_get_agg(mem, (void **)&res) != 0)
+	if (mem_get_agg(mem, (void **)&res) != 0)
 		return;
 	if (!mem_is_null(res))
 		sql_result_value(context, res);