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 2EC6024240
	for <tarantool-patches@freelists.org>; Mon, 11 Feb 2019 08:39:44 -0500 (EST)
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 dsJ6SBW3gmsW for <tarantool-patches@freelists.org>;
	Mon, 11 Feb 2019 08:39:44 -0500 (EST)
Received: from smtp56.i.mail.ru (smtp56.i.mail.ru [217.69.128.36])
	(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 DB7D3240CF
	for <tarantool-patches@freelists.org>; Mon, 11 Feb 2019 08:39:43 -0500 (EST)
Subject: [tarantool-patches] Re: [PATCH] sql: prohibit negative values
 under LIMIT clause
References: <20190204095108.18768-1-szudin@tarantool.org>
 <20190208173709.GI4588@chai>
From: Stanislav Zudin <szudin@tarantool.org>
Message-ID: <8a6ec146-7985-83ab-0b62-8815331d25b1@tarantool.org>
Date: Mon, 11 Feb 2019 16:39:40 +0300
MIME-Version: 1.0
In-Reply-To: <20190208173709.GI4588@chai>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
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: <tarantool-patches-request@freelists.org?Subject=unsubscribe>
List-software: Ecartis version 1.0.0
List-Id: tarantool-patches <tarantool-patches.freelists.org>
List-subscribe: <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: Konstantin Osipov <kostja@tarantool.org>, tarantool-patches@freelists.org
Cc: korablev@tarantool.org



On 08.02.2019 20:37, Konstantin Osipov wrote:
> * Stanislav Zudin <szudin@tarantool.org> [19/02/04 14:32]:
>> +	const char *negativeLimitError =
>> +			"Only positive numbers allowed "
>> +			"in the LIMIT clause";
> We should use diag_set, to ensure error are visible to the
> documentation team and can be localized in the future.
In the recent version the error is reported by means of VDBE (Halt opcode).

>
>>   	if (p->iLimit)
>>   		return;
>>   
>> @@ -2098,6 +2101,13 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
>>   		v = sqlite3GetVdbe(pParse);
>>   		assert(v != 0);
>>   		if (sqlite3ExprIsInteger(p->pLimit, &n)) {
>> +		    if (n < 0) {
>> +                sqlite3VdbeAddOp4(v, OP_Halt,
>> +                                  SQLITE_MISMATCH,
>> +                                  0, 0, negativeLimitError,
>> +                                  P4_STATIC);
>> +            }
>> +
> I don't think we should do any check for  limit values during
> parsing, since limit should allow bind parameters anyway. This
> could should become superfluous if the patch is done right.
This check's been removed in the recent version.
Both LIMIT and OFFSET expressions are being evaluated by VDBE regardless 
of their complexity.
>> +			int lPosOffsetValue = sqlite3VdbeMakeLabel(v);
>> +			const char *negativeOffsetError =
>> +					"Only positive numbers allowed "
>> +					"in the OFFSET clause";
> Ugh, same message is copied multiple times?!
>
Nope, one message is dedicated to the LIMIT, while the another - to 
OFFSET clause.