From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 47739261C8 for ; Fri, 8 Feb 2019 05:17:05 -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 HtjfIyyioo3T for ; Fri, 8 Feb 2019 05:17:05 -0500 (EST) Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94]) (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 A132124709 for ; Fri, 8 Feb 2019 05:17:04 -0500 (EST) Subject: [tarantool-patches] Re: [PATCH] sql: remove useless pragmas References: <20190206121822.9534-1-szudin@tarantool.org> From: Stanislav Zudin Message-ID: Date: Fri, 8 Feb 2019 13:17:02 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit 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: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, "n.pettik" On 06.02.2019 16:21, n.pettik wrote: > >> On 6 Feb 2019, at 15:18, Stanislav Zudin 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.