[tarantool-patches] Re: [PATCH] sql: remove useless pragmas

[Stanislav Zudin <szudin@tarantool.org>]

On 06.02.2019 16:21, n.pettik wrote:
>
>> On 6 Feb 2019, at 15:18, Stanislav Zudin <szudin@tarantool.org> wrote:
>>
>> The pragmas "query_only" and "read_uncommitted" didn't affect anything and were removed.
> Nit: please, fit commit message into 72 symbols.
Mea culpa
>
> What about “busy_timeout”? As I see from code it is also useless.
> It may require to delete some code more code.
It calls sqlite_busy_handler. Does this feature work?
>
> Moreover, last two arguments of index_list are still unused:
>
>     386	void
>     387	sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
>     388	{
> ...
>     398			sqlite3VdbeMultiLoad(v, 1, "isisi", i, idx->def->name,
>     399					     idx->def->opts.is_unique);
>
> Function expects 5 args (int-string-int-string-int), but only three are passed.
> Let’s remove them as well.
Fixed.
>
> Finally, consider comment from P. Gulutzan and my answer attached to ticket.
> In a nutshell, at least two points (except for notes above) should be fixed:
> a) Raise an error if index_list accepts name of unexisting index;
> b) Make sql_default_engine accept only string, not id.
Done.
>
> You may expand current patch into patch-set.
>
>> Closes #3733
>> ---
>> Branch: https://github.com/tarantool/tarantool/tree/stanztt/gh-3733-obsolete-pragmas
>> Issue: https://github.com/tarantool/tarantool/issues/3733
>>
>> src/box/sql/pragma.h    | 10 ----------
>> src/box/sql/sqliteInt.h |  3 ---
>> 2 files changed, 13 deletions(-)
>>
>> diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
>> index e60801608..6b27f83c2 100644
>> --- a/src/box/sql/pragma.h
>> +++ b/src/box/sql/pragma.h
>> @@ -153,16 +153,6 @@ static const PragmaName aPragmaName[] = {
>> 	 /* iArg:      */ 0},
>> #endif
>> #if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
>> -	{ /* zName:     */ "query_only",
>> -	 /* ePragTyp:  */ PragTyp_FLAG,
>> -	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
>> -	 /* ColNames:  */ 0, 0,
>> -	 /* iArg:      */ SQLITE_QueryOnly},
>> -	{ /* zName:     */ "read_uncommitted",
>> -	 /* ePragTyp:  */ PragTyp_FLAG,
>> -	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
>> -	 /* ColNames:  */ 0, 0,
> Personally I would add very simple tests to make sure
> that these pragmas can’t be used anymore.
>
Agreed.