From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 9293746970E for ; Mon, 30 Dec 2019 20:14:37 +0300 (MSK) Date: Mon, 30 Dec 2019 20:14:36 +0300 From: Alexander Turenko Message-ID: <20191230171436.qyzuyteiux25ehhl@tkn_work_nb> References: <20191230030904.ajxcgod553i424e2@tkn_work_nb> <20191230095541.GA21699@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191230095541.GA21699@tarantool.org> 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: Mergen Imeev Cc: tarantool-patches@dev.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 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: