[tarantool-patches] Re: [PATCH] sql: prohibit negative values under LIMIT clause

[Stanislav Zudin <szudin@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.