From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tarantool-patches-bounce@freelists.org>
Received: from localhost (localhost [127.0.0.1])
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 1D77D29F82
	for <tarantool-patches@freelists.org>; Tue, 23 Apr 2019 15:58:11 -0400 (EDT)
Received: from turing.freelists.org ([127.0.0.1])
	by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id Qzi2PP_IGo8V for <tarantool-patches@freelists.org>;
	Tue, 23 Apr 2019 15:58:11 -0400 (EDT)
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 5D7B729284
	for <tarantool-patches@freelists.org>; Tue, 23 Apr 2019 15:58:10 -0400 (EDT)
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\))
Subject: [tarantool-patches] Re: [PATCH 2/9] sql: disallow text values
 participate in sum() aggregate
From: "n.pettik" <korablev@tarantool.org>
In-Reply-To: <6484927f-90fc-666f-7516-f11005f09d2c@tarantool.org>
Date: Tue, 23 Apr 2019 22:58:07 +0300
Content-Transfer-Encoding: quoted-printable
Message-Id: <EC5E066E-FCF3-4047-8745-C0A063CC78CC@tarantool.org>
References: <cover.1555252410.git.korablev@tarantool.org>
 <cf6661047c5e04da60569bd4118cafc1452401b8.1555252410.git.korablev@tarantool.org>
 <829e1d67-9335-65a0-714f-a5684dc3aab5@tarantool.org>
 <D4EBA48D-A684-4308-940E-8C27C1A6D800@tarantool.org>
 <6484927f-90fc-666f-7516-f11005f09d2c@tarantool.org>
Sender: tarantool-patches-bounce@freelists.org
Errors-to: tarantool-patches-bounce@freelists.org
Reply-To: tarantool-patches@freelists.org
List-Help: <mailto:ecartis@freelists.org?Subject=help>
List-Unsubscribe: <mailto:tarantool-patches-request@freelists.org?Subject=unsubscribe>
List-software: Ecartis version 1.0.0
List-Id: tarantool-patches <tarantool-patches.freelists.org>
List-Subscribe: <mailto:tarantool-patches-request@freelists.org?Subject=subscribe>
List-Owner: <mailto:>
List-post: <mailto:tarantool-patches@freelists.org>
List-Archive: <http://www.freelists.org/archives/tarantool-patches>
To: tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>


>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>> index b86a95d9a..9adfeec67 100644
>> --- a/src/box/sql/func.c
>> +++ b/src/box/sql/func.c
>> @@ -1495,24 +1495,34 @@ static void
>> sumStep(sql_context * context, int argc, sql_value ** argv)
>> {
>> 	SumCtx *p;
>> -	int type;
>> 	assert(argc =3D=3D 1);
>> 	UNUSED_PARAMETER(argc);
>> 	p =3D sql_aggregate_context(context, sizeof(*p));
>> -	type =3D sql_value_numeric_type(argv[0]);
>> -	if (p && type !=3D SQL_NULL) {
>> -		p->cnt++;
>> -		if (type =3D=3D SQL_INTEGER) {
>> -			i64 v =3D sql_value_int64(argv[0]);
>> -			p->rSum +=3D v;
>> -			if ((p->approx | p->overflow) =3D=3D 0
>> -			    && sqlAddInt64(&p->iSum, v)) {
>> -				p->overflow =3D 1;
>> -			}
>> -		} else {
>> -			p->rSum +=3D sql_value_double(argv[0]);
>> -			p->approx =3D 1;
>> +	assert(p !=3D NULL);
>=20
> Why are you sure, that p !=3D NULL? sql_aggregate_context()
> on first invocation allocates memory, and it can fail.

Well, theoretically speaking - yes, it can. You=E2=80=99ve fixed that
in review fix, so skipped.

>> +	int type =3D sql_value_type(argv[0]);
>> +	if (type =3D=3D SQL_NULL)
>> +		return;
>=20
> I've fixed the comment above and also I see, that sumStep is
> rewritten almost completely, so it is time to convert it to
> Tarantool style. See the diff below and on the branch in a
> separate commit.

Thx. Diff is OK, applied.