From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 9EE00206F6 for ; Tue, 26 Jun 2018 13:09:38 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ML35if6kaceb for ; Tue, 26 Jun 2018 13:09:38 -0400 (EDT) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 38017206EB for ; Tue, 26 Jun 2018 13:09:38 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/2] sql: introduce pragma sql_default_engine References: <869f48ed-cf99-7e0a-f89b-8431dee2f72c@tarantool.org> <485804b9-8659-17a1-aaa9-cebc1d0e2893@tarantool.org> <9085644e-030f-b1bc-6928-4819bc5ffed7@tarantool.org> From: Kirill Shcherbatov Message-ID: Date: Tue, 26 Jun 2018 20:09:36 +0300 MIME-Version: 1.0 In-Reply-To: <9085644e-030f-b1bc-6928-4819bc5ffed7@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy > 1. It was a rhetorical question. Please, remove it in the way > described in comment 6. ok. > 2. I do not see the new patch version. Please, put it after fixes. appended. My fault. I was confused by \set language. > > Please, do not skip the rest of the comment. I will paste it > here for you: > >> Maybe we should ask in a big red chat about this default engine >> for each language unless we will have to set default engine like >> this when use iproto: >> >> c = netbox.connect(uri) >> c:execute('PRAGMA default_engine = "memtx"') >> c:eval('box.space.default_engine = "memtx"') >> ... -- etc for each language. >> >> And it breaks our "celebratory interoperability" so much >> glorified on T+ conf. >> >> Maybe it is a good question for Gulutzan. > > Please, do what I said here. I've discussed this with Kirill. He confirmed to use this setting only in SQL. > 4. Here strcpy is enough. You do not need any formatting here, > and you know, that engine name fits in space_def.engine_name. Ok. > 5. Please, do not use neither _t nor _type for enums. sql_storage_engine > > 6. Why do you need strcasecmp here? I do not think, > that we should allow 'MeMtX' or 'Memtx' or something. Only > 'memtx' (and 'vinyl'). And in such a case you can declare > sql_storage_engine_strs and use STR2ENUM like it is done > for other enums visible to user. Ok. ================================================== Part of #2199. @TarantoolBot document Title: new pragma sql_default_engine Now it is allowed to create vinyl spaces using special pragma setting default engine for SQL requests. This config is stored in user_session() and affect *only* tables been created with SQL. Example: \set language sql pragma sql_default_engine='vinyl'; CREATE TABLE t3(a primary key,b,c); --- src/box/schema_def.h | 7 +++++++ src/box/session.cc | 1 + src/box/session.h | 2 ++ src/box/sql/build.c | 11 ++++++++++- src/box/sql/pragma.c | 28 ++++++++++++++++++++++++++++ src/box/sql/pragma.h | 6 ++++++ src/box/sql/sqliteInt.h | 2 ++ test/sql-tap/gh-2367-pragma.test.lua | 29 ++++++++++++++++++++++++++++- 8 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/box/schema_def.h b/src/box/schema_def.h index b9a5fa5..e6a7206 100644 --- a/src/box/schema_def.h +++ b/src/box/schema_def.h @@ -243,6 +243,13 @@ enum schema_object_type { schema_object_type_MAX = 8 }; +/** SQL Storage engine. */ +enum sql_storage_engine { + SQL_STORAGE_ENGINE_MEMTX = 0, + SQL_STORAGE_ENGINE_VINYL = 1, + sql_storage_engine_MAX = 2 +}; + enum schema_object_type schema_object_type(const char *name); diff --git a/src/box/session.cc b/src/box/session.cc index e487280..64714cd 100644 --- a/src/box/session.cc +++ b/src/box/session.cc @@ -107,6 +107,7 @@ session_create(enum session_type type) memset(&session->meta, 0, sizeof(session->meta)); session->type = type; session->sql_flags = default_flags; + session->sql_default_engine = SQL_STORAGE_ENGINE_MEMTX; /* For on_connect triggers. */ credentials_init(&session->credentials, guest_user->auth_token, diff --git a/src/box/session.h b/src/box/session.h index 515c30a..df1dcbc 100644 --- a/src/box/session.h +++ b/src/box/session.h @@ -92,6 +92,8 @@ union session_meta { struct session { /** Session id. */ uint64_t id; + /** SQL Tarantool Default storage engine. */ + uint8_t sql_default_engine; /** SQL Connection flag for current user session */ uint32_t sql_flags; enum session_type type; diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 093fea5..5496d29 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -492,6 +492,11 @@ sqlite3PrimaryKeyIndex(Table * pTab) return p; } +const char *sql_storage_engine_strs[] = { + [SQL_STORAGE_ENGINE_MEMTX] = "memtx", + [SQL_STORAGE_ENGINE_VINYL] = "vinyl", +}; + /** * Create and initialize a new SQL Table object. * All memory except table object itself is allocated on region. @@ -510,6 +515,9 @@ sql_table_new(Parse *parser, char *name) if (table == NULL) return NULL; + strcpy(table->def->engine_name, + sql_storage_engine_strs[current_session()->sql_default_engine]); + table->iPKey = -1; table->iAutoIncPKey = -1; table->pSchema = db->pSchema; @@ -1602,7 +1610,8 @@ createSpace(Parse * pParse, int iSpaceId, char *zStmt) sqlite3VdbeAddOp4(v, OP_String8, 0, iFirstCol + 2 /* name */ , 0, sqlite3DbStrDup(pParse->db, p->def->name), P4_DYNAMIC); sqlite3VdbeAddOp4(v, OP_String8, 0, iFirstCol + 3 /* engine */ , 0, - "memtx", P4_STATIC); + sqlite3DbStrDup(pParse->db, p->def->engine_name), + P4_DYNAMIC); sqlite3VdbeAddOp2(v, OP_Integer, p->def->field_count, iFirstCol + 4 /* field_count */ ); sqlite3VdbeAddOp4(v, OP_Blob, zOptsSz, iFirstCol + 5, MSGPACK_SUBTYPE, diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index be6a01c..7a25db6 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -235,6 +235,26 @@ printActivePragmas(struct session *user_session) } } +/** + * Set tarantool backend default engine for SQL interface. + * @param engine_name to set default. + * @retval -1 on error. + * @retval 0 on success. + */ +static int +sql_default_engine_set(const char *engine_name) +{ + enum sql_storage_engine engine = + STR2ENUM(sql_storage_engine, engine_name); + if (engine == sql_storage_engine_MAX) { + diag_set(ClientError, ER_NO_SUCH_ENGINE, engine_name); + return -1; + } + current_session()->sql_default_engine = engine; + return 0; +} + + /* * Process a pragma statement. * @@ -868,6 +888,14 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ } break; } + case PragTyp_DEFAULT_ENGINE: { + if (sql_default_engine_set(zRight) != 0) { + pParse->rc = SQL_TARANTOOL_ERROR; + pParse->nErr++; + goto pragma_out; + } + break; + } /* * PRAGMA busy_timeout * PRAGMA busy_timeout = N * * diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h index 06b7eea..795c98c 100644 --- a/src/box/sql/pragma.h +++ b/src/box/sql/pragma.h @@ -17,6 +17,7 @@ #define PragTyp_STATS 15 #define PragTyp_TABLE_INFO 17 #define PragTyp_PARSER_TRACE 24 +#define PragTyp_DEFAULT_ENGINE 25 /* Property flags associated with various pragma. */ #define PragFlg_NeedSchema 0x01 /* Force schema load before running */ @@ -247,6 +248,11 @@ static const PragmaName aPragmaName[] = { /* ColNames: */ 0, 0, /* iArg: */ SQLITE_ShortColNames}, #endif + { /* zName: */ "sql_default_engine", + /* ePragTyp: */ PragTyp_DEFAULT_ENGINE, + /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, + /* ColNames: */ 0, 0, + /* iArg: */ 0}, #if !defined(SQLITE_OMIT_FLAG_PRAGMAS) #if defined(SQLITE_DEBUG) { /* zName: */ "sql_trace", diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index d6ae5c2..918fe90 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -4804,4 +4804,6 @@ vdbe_emit_halt_if_exists(struct Parse *parser, int space_id, int index_id, const char *name_src, int tarantool_error_code, const char *error_src, bool no_error); +extern const char *sql_storage_engine_strs[]; + #endif /* SQLITEINT_H */ diff --git a/test/sql-tap/gh-2367-pragma.test.lua b/test/sql-tap/gh-2367-pragma.test.lua index a41a026..6d2e73d 100755 --- a/test/sql-tap/gh-2367-pragma.test.lua +++ b/test/sql-tap/gh-2367-pragma.test.lua @@ -1,7 +1,7 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(1) +test:plan(4) test:do_catchsql_test( "pragma-1.3", @@ -11,4 +11,31 @@ test:do_catchsql_test( 1, "no such pragma: KEK" }) +--- +--- gh-2199: SQL default engine pragma +--- +test:do_catchsql_test( + "pragma-2.1", + [[ + pragma sql_default_engine='creepy'; + ]], { + 1, "Space engine 'creepy' does not exist" +}) + +test:do_catchsql_test( + "pragma-2.2", + [[ + pragma sql_default_engine='vinyl'; + ]], { + 0 +}) + +test:do_catchsql_test( + "pragma-2.3", + [[ + pragma sql_default_engine='memtx'; + ]], { + 0 +}) + test:finish_test() -- 2.7.4