From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 B4CA846970E for ; Mon, 30 Dec 2019 12:55:43 +0300 (MSK) Date: Mon, 30 Dec 2019 12:55:41 +0300 From: Mergen Imeev Message-ID: <20191230095541.GA21699@tarantool.org> References: <20191230030904.ajxcgod553i424e2@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191230030904.ajxcgod553i424e2@tkn_work_nb> Subject: Re: [Tarantool-patches] [PATCH v2 test-run 1/1] sql: add another way to change SQL default engine List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org 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 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')