From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2C13A46970E for ; Fri, 27 Dec 2019 18:26:41 +0300 (MSK) References: <734163151611e2da16f7901b42f9829d63f26053.1577445318.git.imeevma@gmail.com> From: Vladislav Shpilevoy Message-ID: Date: Fri, 27 Dec 2019 18:26:39 +0300 MIME-Version: 1.0 In-Reply-To: <734163151611e2da16f7901b42f9829d63f26053.1577445318.git.imeevma@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v5 4/5] sql: remove control pragmas List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! I've pushed my review fixes on top of this commit. See it below and on the branch. If you agree, then squash. Otherwise lets discuss. ================================================================================ commit 66b927c57c84b49ef2244efb49c0986928fb8e2b Author: Vladislav Shpilevoy Date: Fri Dec 27 17:32:35 2019 +0300 Review fixes 4/5 diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index d859d6380..f4d6e52cd 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3771,7 +3771,6 @@ sql_space_def_check_format(const struct space_def *space_def); void sqlDetach(Parse *, Expr *); int sqlAtoF(const char *z, double *, int); int sqlGetInt32(const char *, int *); -int sqlAtoi(const char *); /** * Return number of symbols in the given string. @@ -3959,8 +3958,6 @@ int sql_rem_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, int64_t *res, bool *is_res_neg); -u8 sqlGetBoolean(const char *z, u8); - const void *sqlValueText(sql_value *); int sqlValueBytes(sql_value *); void sqlValueSetStr(sql_value *, int, const void *, diff --git a/src/box/sql/util.c b/src/box/sql/util.c index 0e115accf..f908e9ced 100644 --- a/src/box/sql/util.c +++ b/src/box/sql/util.c @@ -495,8 +495,6 @@ sql_atoi64(const char *z, int64_t *val, bool *is_neg, int length) * This routine accepts both decimal and hexadecimal notation for integers. * * Any non-numeric characters that following zNum are ignored. - * This is different from sqlAtoi64() which requires the - * input number to be zero-terminated. */ int sqlGetInt32(const char *zNum, int *pValue) @@ -553,19 +551,6 @@ sqlGetInt32(const char *zNum, int *pValue) return 1; } -/* - * Return a 32-bit integer value extracted from a string. If the - * string is not an integer, just return 0. - */ -int -sqlAtoi(const char *z) -{ - int x = 0; - if (z) - sqlGetInt32(z, &x); - return x; -} - /* * The variable-length integer encoding is as follows: * diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 8e12c37a0..c5c0a9790 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -4778,8 +4778,8 @@ case OP_Program: { /* jump */ /* If the p5 flag is clear, then recursive invocation of triggers is * disabled for backwards compatibility (p5 is set if this sub-program - * is really a trigger, not a foreign key action, and the flag set - * and cleared by the "PRAGMA recursive_triggers" command is clear). + * is really a trigger, not a foreign key action, and the setting + * 'recursive_triggers' is not set). * * It is recursive invocation of triggers, at the SQL level, that is * disabled. In some cases a single trigger may generate more than one diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h index 1e585d89d..0f2703091 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -199,11 +199,6 @@ int sqlVdbeAddOp4(Vdbe *, int, int, int, int, const char *zP4, int); int sqlVdbeAddOp4Dup8(Vdbe *, int, int, int, int, const u8 *, int); int sqlVdbeAddOp4Int(Vdbe *, int, int, int, int, int); void sqlVdbeEndCoroutine(Vdbe *, int); -#if defined(SQL_DEBUG) && !defined(SQL_TEST_REALLOC_STRESS) -void sqlVdbeVerifyNoResultRow(Vdbe * p); -#else -#define sqlVdbeVerifyNoResultRow(A) -#endif void sqlVdbeChangeOpcode(Vdbe *, u32 addr, u8); void sqlVdbeChangeP1(Vdbe *, u32 addr, int P1); void sqlVdbeChangeP2(Vdbe *, u32 addr, int P2); diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 5b4bc0182..43e0566bb 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -533,24 +533,6 @@ sqlVdbeCurrentAddr(Vdbe * p) return p->nOp; } -/* - * Verify that the VM passed as the only argument does not contain - * an OP_ResultRow opcode. Fail an assert() if it does. This is used - * by code in pragma.c to ensure that the implementation of certain - * pragmas comports with the flags specified in the mkpragmatab.tcl - * script. - */ -#if defined(SQL_DEBUG) && !defined(SQL_TEST_REALLOC_STRESS) -void -sqlVdbeVerifyNoResultRow(Vdbe * p) -{ - int i; - for (i = 0; i < p->nOp; i++) { - assert(p->aOp[i].opcode != OP_ResultRow); - } -} -#endif - /* * This function returns a pointer to the array of opcodes associated with * the Vdbe passed as the first argument. It is the callers responsibility diff --git a/test/sql-tap/lua/sqltester.lua b/test/sql-tap/lua/sqltester.lua index de5ef6b8b..9b0218e63 100644 --- a/test/sql-tap/lua/sqltester.lua +++ b/test/sql-tap/lua/sqltester.lua @@ -413,7 +413,7 @@ box.cfg{ } local engine = test_run and test_run:get_cfg('engine') or 'memtx' -_ = box.space._session_settings:update('sql_default_engine', {{'=', 2, engine}}) +box.space._session_settings:update('sql_default_engine', {{'=', 2, engine}}) function test.engine(self) return engine diff --git a/test/sql-tap/select1.test.lua b/test/sql-tap/select1.test.lua index f309e1e73..bea5b9c7c 100755 --- a/test/sql-tap/select1.test.lua +++ b/test/sql-tap/select1.test.lua @@ -2,6 +2,12 @@ test = require("sqltester") test:plan(173) +function set_full_column_names(value) + box.space._session_settings:update('sql_full_column_names', { + {'=', 2, value} + }) +end + --!./tcltestrunner.lua -- ["set","testdir",[["file","dirname",["argv0"]]]] -- ["source",[["testdir"],"\/tester.tcl"]] @@ -916,7 +922,7 @@ test:do_catchsql2_test( test:do_test( "select1-6.1.1", function() - box.space._session_settings:update('sql_full_column_names', {{'=', 2, true}}) + set_full_column_names(true) return test:catchsql2 "SELECT f1 FROM test1 ORDER BY f2" end, { -- @@ -952,7 +958,7 @@ test:do_test( msg = test:execsql2 "SELECT DISTINCT * FROM test1 WHERE f1==11" end) v = v == true and {0} or {1} - box.space._session_settings:update('sql_full_column_names', {{'=', 2, false}}) + set_full_column_names(false) return table.insert(v,msg) or v end, { -- @@ -1043,13 +1049,13 @@ test:do_catchsql2_test( test:do_test( "select1-6.5.1", function() - box.space._session_settings:update('sql_full_column_names', {{'=', 2, true}}) + set_full_column_names(true) local msg v = pcall( function () msg = test:execsql2 "SELECT test1.f1+F2 FROM test1 ORDER BY f2" end) v = v == true and {0} or {1} - box.space._session_settings:update('sql_full_column_names', {{'=', 2, false}}) + set_full_column_names(false) return table.insert(v,msg) or v end, { -- @@ -1123,7 +1129,7 @@ test:do_catchsql2_test( test:do_test( "select1-6.9.3", function() - box.space._session_settings:update('sql_full_column_names', {{'=', 2, false}}) + set_full_column_names(false) return test:execsql2 [[ SELECT test1 . f1, test1 . f2 FROM test1 LIMIT 1 ]] @@ -1136,7 +1142,7 @@ test:do_test( test:do_test( "select1-6.9.4", function() - box.space._session_settings:update('sql_full_column_names', {{'=', 2, true}}) + set_full_column_names(true) return test:execsql2 [[ SELECT test1 . f1, test1 . f2 FROM test1 LIMIT 1 ]] @@ -1149,7 +1155,7 @@ test:do_test( test:do_test( "select1-6.9.5", function() - box.space._session_settings:update('sql_full_column_names', {{'=', 2, true}}) + set_full_column_names(true) return test:execsql2 [[ SELECT 123.45; ]] @@ -1228,7 +1234,7 @@ test:do_execsql2_test( test:do_test( "select1-6.9.11", function() - box.space._session_settings:update('sql_full_column_names', {{'=', 2, true}}) + set_full_column_names(true) return test:execsql2 [[ SELECT a.f1, b.f2 FROM test1 a, test1 b LIMIT 1 ]] @@ -1251,7 +1257,7 @@ test:do_execsql2_test( test:do_test( "select1-6.9.13", function() - box.space._session_settings:update('sql_full_column_names', {{'=', 2, false}}) + set_full_column_names(false) return test:execsql2 [[ SELECT a.f1, b.f1 FROM test1 a, test1 b LIMIT 1 ]] @@ -1274,7 +1280,7 @@ test:do_execsql2_test( test:do_test( "select1-6.9.15", function() - box.space._session_settings:update('sql_full_column_names', {{'=', 2, true}}) + set_full_column_names(true) return test:execsql2 [[ SELECT a.f1, b.f1 FROM test1 a, test1 b LIMIT 1 ]] @@ -1294,7 +1300,7 @@ test:do_execsql2_test( -- }) -box.space._session_settings:update('sql_full_column_names', {{'=', 2, false}}) +set_full_column_names(false) test:do_catchsql2_test( "select1-6.10", [[ diff --git a/test/sql/transitive-transactions.result b/test/sql/transitive-transactions.result index e5868d7b0..91c35a309 100644 --- a/test/sql/transitive-transactions.result +++ b/test/sql/transitive-transactions.result @@ -87,7 +87,7 @@ box.space.PARENT:select(); --- - - [1, 1] ... --- Make sure that 'PRAGMA defer_foreign_keys' works. +-- Make sure that setting 'defer_foreign_keys' works. -- box.execute('DROP TABLE child;') box.execute('CREATE TABLE child(id INT PRIMARY KEY, x INT REFERENCES parent(y))') diff --git a/test/sql/transitive-transactions.test.lua b/test/sql/transitive-transactions.test.lua index f4df716c3..5565de7fe 100644 --- a/test/sql/transitive-transactions.test.lua +++ b/test/sql/transitive-transactions.test.lua @@ -45,7 +45,7 @@ fk_violation_3(); box.space.CHILD:select(); box.space.PARENT:select(); --- Make sure that 'PRAGMA defer_foreign_keys' works. +-- Make sure that setting 'defer_foreign_keys' works. -- box.execute('DROP TABLE child;') box.execute('CREATE TABLE child(id INT PRIMARY KEY, x INT REFERENCES parent(y))')