Re: [Tarantool-patches] [PATCH v2 test-run 1/1] sql: add another way to change SQL default engine

[Alexander Turenko <alexander.turenko@tarantool.org>]

Applied changes after voice discussion with Mergen (see below). Pushed
to test-run's master. I'll update test-run in tarantool's branches
(master, 2.2 and 1.10) soon.

WBR, Alexander Turenko.

>  "engine" value has a special meaning for *.test.sql files: if it is "memtx" or
> -"vinyl", then the corresponding engine will be set using "pragma
> -sql_default_engine='memtx|vinyl'" command before executing commands from a test
> -file.
> +"vinyl", then the corresponding engine will be set using either "pragma
> +sql_default_engine='memtx|vinyl'" or "UPDATE \\"_session_settings\\" SET
> +\\"value\\" = 'memtx|vinyl' WHERE \\"name\\" = 'sql_default_engine';" command
> +before executing commands from a test file.

I placed "UPDATE ..." first, "pragma ..." second and extracted commands
from the text itself.

> diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py
> index 6f71b5b..07cbf0d 100644
> --- a/lib/tarantool_server.py
> +++ b/lib/tarantool_server.py
> @@ -145,6 +145,14 @@ class LuaTest(Test):
>              return True
>  
>          engine = self.run_params['engine']
> +        command = ("UPDATE \"_session_settings\" SET \"value\" = '{}' " +
> +                   "WHERE \"name\" = 'sql_default_engine';").format(engine)
> +        result = self.send_command(command, ts, 'sql')
> +        result = result.replace('\r\n', '\n')
> +        if result != '---\n- row_count: 1\n...\n' and result != "---\n- " +\
> +           "null\n- Space '_session_settings' does not exist\n...\n":
> +            sys.stdout.write(result)
> +            return False
>          command = "pragma sql_default_engine='{}'".format(engine)
>          result = self.send_command(command, ts, 'sql')
>          result = result.replace('\r\n', '\n')

We discussed in with Mergen voicely. I proposed to eliminate hardcoding
of "Space '_session_settings' does not exist" error just in case: if it
was changed somehow (say, an engine or other info will be added), we'll
need to update test-run again. I would try to avoid it.

Mergen's concern was against ignoring of the first error message: this
may hid useful information in case of some failure.

I think we can just show both results in case of fail and eliminate the
check against 'Space <...> does not exist' error: proceed with 2nd
command after any fail of 1nd.

Resulting patch is below. I verified it with both master
(e13a74e4383744a7ff5ddec83c74e13c82e417dc) and
imeevma/gh-4511-pragma-replaced-by-set branch
(dbe63f9c561bbac3748fcfa22bf09c5b0532d7fd) and it seems everything is
okay.

----

commit dc2cdb554f37efc2d8845e4e0350f53c451f32a7 (HEAD -> master)
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Sun Dec 29 16:48:29 2019 +0300

    sql: add another way to change SQL default engine
    
    Currently, the pragma sql_default_engine is used in the *.test.sql
    tests to change the default SQL engine. But in the issue
    tarantool/tarantool#4511, this pragma will be removed. To avoid
    failures in these tests because of this, in addition to the
    current one, another way to change this parameter was implemented.

diff --git a/README.md b/README.md
index aefeeff..48803bf 100644
--- a/README.md
+++ b/README.md
@@ -69,9 +69,15 @@ engine = test_run:get_cfg('engine')
 ```
 
 "engine" value has a special meaning for *.test.sql files: if it is "memtx" or
-"vinyl", then the corresponding engine will be set using "pragma
-sql_default_engine='memtx|vinyl'" command before executing commands from a test
-file.
+"vinyl", then the corresponding default engine will be set before executing
+commands from a test file. An engine is set with the following commands:
+
+```sql
+UPDATE "_session_settings" SET "value" = 'memtx|vinyl' WHERE "name" = 'sql_default_engine'
+pragma sql_default_engine='memtx|vinyl'
+```
+
+If the first fails, then the second will be executed. When both fails, fail the test.
 
 #### Python
 
diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py
index 6f71b5b..fd102b9 100644
--- a/lib/tarantool_server.py
+++ b/lib/tarantool_server.py
@@ -145,14 +145,27 @@ class LuaTest(Test):
             return True
 
         engine = self.run_params['engine']
-        command = "pragma sql_default_engine='{}'".format(engine)
-        result = self.send_command(command, ts, 'sql')
-        result = result.replace('\r\n', '\n')
-        if result != '---\n- row_count: 0\n...\n':
-            sys.stdout.write(result)
-            return False
 
-        return True
+        # Probe the new way. Pass through on any error.
+        command_new = ("UPDATE \"_session_settings\" SET \"value\" = '{}' " +
+                       "WHERE \"name\" = 'sql_default_engine'").format(engine)
+        result_new = self.send_command(command_new, ts, 'sql')
+        result_new = result_new.replace('\r\n', '\n')
+        if result_new == '---\n- row_count: 1\n...\n':
+            return True
+
+        # Probe the old way. Fail the test on an error.
+        command_old = "pragma sql_default_engine='{}'".format(engine)
+        result_old = self.send_command(command_old, ts, 'sql')
+        result_old = result_old.replace('\r\n', '\n')
+        if result_old == '---\n- row_count: 0\n...\n':
+            return True
+
+        sys.stdout.write(command_new)
+        sys.stdout.write(result_new)
+        sys.stdout.write(command_old)
+        sys.stdout.write(result_old)
+        return False
 
     def set_language(self, ts, language):
         for conn in ts.curcon: