Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev <imeevma@tarantool.org>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 test-run 1/1] sql: add another way to change SQL default engine
Date: Mon, 30 Dec 2019 12:55:41 +0300	[thread overview]
Message-ID: <20191230095541.GA21699@tarantool.org> (raw)
In-Reply-To: <20191230030904.ajxcgod553i424e2@tkn_work_nb>

Hi! Thank you for review. New patch below.

On Mon, Dec 30, 2019 at 06:09:04AM +0300, Alexander Turenko wrote:
> > >> diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py
> > >> index 6f71b5b..20a398e 100644
> > >> --- a/lib/tarantool_server.py
> > >> +++ b/lib/tarantool_server.py
> > >> @@ -145,12 +145,19 @@ 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
> > >> +
> > >> +        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')
> > >> +
> > >> +        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_new != '---\n- row_count: 1\n...\n':
> > >> +            if result_old != '---\n- row_count: 0\n...\n':
> > >> +                sys.stdout.write(result_old)
> > >> +                return False
> > >
> > > I would make it like so:
> > >
> > > 1. Send "pragma ...", await a result.
> > >    - If succeeds, return True.
> > >    - Otherwise proceed with the step 2.
> > > 2. Send "UPDATE ...", await a result.
> > >    - If succeeds, return True.
> > >    - Otherwise return False.
> > >
> > Fixed.
> 
> Ouch, occasionally I misdirected you. Sorry. I intend to say that we
> should send the new-style command first and only then (if it fails) send
> old-style "pragma ..." command.
> 
> This way we'll optimize the case when the new tarantool is tested that
> will be most often case after some time.
> 
Fixed.

> > > This way we'll save one extra hop on a fresh tarantool. Not so big
> > > overhead for an action that is run once for a test, but we can overcome
> > > it and pay nothing, so it looks worthful for me.

New patch:

From fd9a8af20393d6df3622ab1ddd8884b7333d1c11 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Sun, 29 Dec 2019 16:48:29 +0300
Subject: [PATCH] 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..781d003 100644
--- a/README.md
+++ b/README.md
@@ -69,9 +69,10 @@ 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 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.
 
 #### Python
 
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')

  reply	other threads:[~2019-12-30  9:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-29 14:20 imeevma
2019-12-30  3:09 ` Alexander Turenko
2019-12-30  9:55   ` Mergen Imeev [this message]
2019-12-30 17:14     ` Alexander Turenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191230095541.GA21699@tarantool.org \
    --to=imeevma@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 test-run 1/1] sql: add another way to change SQL default engine' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox