[Tarantool-patches] [PATCH v5 4/5] sql: remove control pragmas

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Fri Dec 27 18:26:39 MSK 2019


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 <v.shpilevoy at tarantool.org>
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, {
         -- <select1-6.1.1>
@@ -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, {
         -- <select1-6.1.4>
@@ -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, {
         -- <select1-6.5.1>
@@ -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(
         -- </select1-6.9.16>
     })
 
-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))')


More information about the Tarantool-patches mailing list