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 9FC60401A41 for ; Fri, 25 Oct 2019 18:45:45 +0300 (MSK) From: imeevma@tarantool.org Date: Fri, 25 Oct 2019 18:45:44 +0300 Message-Id: <8444f08e9a52ec809208ae7e85a69b44014dc215.1572014706.git.imeevma@gmail.com> In-Reply-To: References: Subject: [Tarantool-patches] [PATCH v2 3/5] sql: introduce SET statement List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org Thank you for review! My answers and new patch below. On 10/19/19 1:08 AM, Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 3 comments below. > > On 17/10/2019 16:40, imeevma@tarantool.org wrote: >> This patch creates the SET command for SQL, which will be used >> instead of pragmas that modify SQL settings. >> >> Part of #4511 >> >> @TarantoolBot document >> Title: SET SQL command >> The SET SQL command is used to change SQL settings. > > 1. Please, provide SET syntax description. And say, that pragmas > are dropped. And which of them are dropped, which are converted > into SET. > Fixed. >> >> Currently available SQL settings: >> 'defer_foreign_keys' >> 'full_column_names' >> 'recursive_triggers' >> 'reverse_unordered_selects' >> 'sql_compound_select_limit' >> 'sql_default_engine' >> >> In addition, SQL debugging settings can also be changed using this >> command in the debug build: >> 'parser_trace' >> 'select_trace' >> 'sql_trace' >> 'vdbe_addoptrace' >> 'vdbe_debug' >> 'vdbe_eqp' >> 'vdbe_listing' >> 'vdbe_trace' >> 'where_trace' >> >> Example of usage: >> SET full_column_names = true; >> SET sql_compound_select_limit(10); >> SET sql_default_engine = 'memtx'; >> --- >> src/box/sql/build.c | 60 +++++++++++++++++++++ >> src/box/sql/parse.y | 8 +++ >> src/box/sql/sqlInt.h | 14 +++++ >> test/sql/sql-debug.result | 127 ++++++++++++++++++++++++++++++++++++++++++++ >> test/sql/sql-debug.test.lua | 26 +++++++++ >> 5 files changed, 235 insertions(+) >> >> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y >> index ed59a87..7bea68d 100644 >> --- a/src/box/sql/parse.y >> +++ b/src/box/sql/parse.y >> @@ -1535,6 +1535,14 @@ cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). { >> sql_drop_index(pParse); >> } >> >> +///////////////////////////// The SET command //////////////////////////////// >> +cmd ::= SET nm(X) EQ term(Y). { >> + sql_set_settings(pParse,&X,Y.pExpr); >> +} >> +cmd ::= SET nm(X) LP term(Y) RP. { >> + sql_set_settings(pParse,&X,Y.pExpr); > > 2. Do we really need both syntax variants? > No, I think. Fixed. >> diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result >> index 2dba684..07542e3 100644 >> --- a/test/sql/sql-debug.result >> +++ b/test/sql/sql-debug.result >> @@ -54,3 +54,130 @@ box.execute('PRAGMA') >> - ['vdbe_trace', 0] >> - ['where_trace', 0] >> ... >> +-- >> +-- gh-4511: make sure that SET works. >> +-- >> +box.execute('SELECT "name" FROM "_vsql_settings";') >> +--- >> +- metadata: >> + - name: name >> + type: string >> + rows: >> + - ['defer_foreign_keys'] >> + - ['full_column_names'] >> + - ['recursive_triggers'] >> + - ['reverse_unordered_selects'] >> + - ['sql_compound_select_limit'] >> + - ['sql_default_engine'] >> + - ['parser_trace'] >> + - ['select_trace'] >> + - ['sql_trace'] >> + - ['vdbe_addoptrace'] >> + - ['vdbe_debug'] >> + - ['vdbe_eqp'] >> + - ['vdbe_listing'] >> + - ['vdbe_trace'] >> + - ['where_trace'] >> +... >> +engine = box.space._vsql_settings:get{'sql_default_engine'}[2] > > 3. Does field access by name work? > Yes, fixed. New patch: >From 8444f08e9a52ec809208ae7e85a69b44014dc215 Mon Sep 17 00:00:00 2001 From: Mergen Imeev Date: Wed, 16 Oct 2019 16:43:10 +0300 Subject: [PATCH] sql: introduce SET statement This patch creates an SQL SET statement. This statement replaces pragmas that can modify SQL settings. List of pragmas that will have the corresponding option in SET: 'defer_foreign_keys' 'full_column_names' 'recursive_triggers' 'reverse_unordered_selects' 'sql_compound_select_limit' 'sql_default_engine' 'parser_trace' 'select_trace' 'sql_trace' 'vdbe_addoptrace' 'vdbe_debug' 'vdbe_eqp' 'vdbe_listing' 'vdbe_trace' 'where_trace' All these pragmas along with the pragmas 'short_column_names' and 'count_changes' will be removed in the next patch. Part of #4511 @TarantoolBot document Title: SQL SET statement SQL SET statement is used to change SQL settings. To change the value of an SQL parameter, use the following syntax: SET = ; Currently available SQL settings: 'sql_defer_foreign_keys' 'sql_full_column_names' 'sql_recursive_triggers' 'sql_reverse_unordered_selects' 'sql_compound_select_limit' 'sql_default_engine' In addition, SQL debugging settings can also be changed using this statement in the debug build: 'sql_parser_trace' 'sql_select_trace' 'sql_trace' 'sql_vdbe_addoptrace' 'sql_vdbe_debug' 'sql_vdbe_eqp' 'sql_vdbe_listing' 'sql_vdbe_trace' 'sql_where_trace' Example of usage: SET full_column_names = true; SET sql_compound_select_limit = 10; SET sql_default_engine = 'memtx'; diff --git a/src/box/sql/build.c b/src/box/sql/build.c index cb97106..3eef4dc 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -3400,3 +3400,63 @@ sql_option_tuple(struct tuple_format *format, int option_id, *result = tuple; return 0; } + +void +sql_set_settings(struct Parse *parse_context, struct Token *name, + struct Expr *value) +{ + int option_id; + struct session *session = current_session(); + char *name_str = sql_name_from_token(sql_get(), name); + if (name_str == NULL) { + parse_context->is_aborted = true; + return; + } + for (option_id = 0; option_id < SQL_OPTION_max; ++option_id) { + if (strcasecmp(sql_options[option_id].name, name_str) == 0) + break; + } + if (option_id == SQL_OPTION_max) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Setting is " + "not found"); + parse_context->is_aborted = true; + return; + } + struct sql_option_metadata *option = &sql_options[option_id]; + if (value->type != option->field_type) { + diag_set(ClientError, ER_INCONSISTENT_TYPES, + field_type_strs[option->field_type], + field_type_strs[value->type]); + parse_context->is_aborted = true; + return; + } + if (value->type == FIELD_TYPE_BOOLEAN) { + bool is_set = value->op == TK_TRUE; + if (is_set) + session->sql_flags |= option->mask; + else + session->sql_flags &= ~option->mask; +#ifndef NDEBUG + if (option_id == SQL_OPTION_PARSER_TRACE) { + if (is_set) + sqlParserTrace(stdout, "parser: "); + else + sqlParserTrace(NULL, NULL); + } +#endif + } else if (option_id == SQL_OPTION_DEFAULT_ENGINE) { + enum sql_storage_engine engine = + STR2ENUM(sql_storage_engine, value->u.zToken); + if (engine == sql_storage_engine_MAX) { + parse_context->is_aborted = true; + diag_set(ClientError, ER_NO_SUCH_ENGINE, + value->u.zToken); + return; + } + current_session()->sql_default_engine = engine; + } else { + assert(option_id == SQL_OPTION_COMPOUND_SELECT_LIMIT); + sql_limit(sql_get(), SQL_LIMIT_COMPOUND_SELECT, + value->u.iValue); + } +} diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 1d0c95f..d0702d7 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -1539,6 +1539,11 @@ cmd ::= DROP INDEX ifexists(E) nm(X) ON fullname(Y). { sql_drop_index(pParse); } +///////////////////////////// The SET command //////////////////////////////// +cmd ::= SET nm(X) EQ term(Y). { + sql_set_settings(pParse,&X,Y.pExpr); +} + ///////////////////////////// The PRAGMA command ///////////////////////////// // cmd ::= PRAGMA nm(X). { diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index c32cacc..0bc8bbd 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4458,4 +4458,18 @@ int sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name, uint32_t *fieldno); +/** + * Set new value for SQL setting. + * + * @param parse_context Parsing context. + * @param name Name of SQL setting to change. + * @param value New values of SQL setting. + * + * @retval 0 on success. + * @retval -1 on error. + */ +void +sql_set_settings(struct Parse *parse_context, struct Token *name, + struct Expr *value); + #endif /* sqlINT_H */ diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result index 2dba684..bfcc046 100644 --- a/test/sql/sql-debug.result +++ b/test/sql/sql-debug.result @@ -54,3 +54,134 @@ box.execute('PRAGMA') - ['vdbe_trace', 0] - ['where_trace', 0] ... +-- +-- gh-4511: make sure that SET works. +-- +box.execute('SELECT "name" FROM "_vsession_settings";') +--- +- metadata: + - name: name + type: string + rows: + - ['sql_compound_select_limit'] + - ['sql_default_engine'] + - ['sql_defer_foreign_keys'] + - ['sql_full_column_names'] + - ['sql_parser_trace'] + - ['sql_recursive_triggers'] + - ['sql_reverse_unordered_selects'] + - ['sql_select_trace'] + - ['sql_trace'] + - ['sql_vdbe_addoptrace'] + - ['sql_vdbe_debug'] + - ['sql_vdbe_eqp'] + - ['sql_vdbe_listing'] + - ['sql_vdbe_trace'] + - ['sql_where_trace'] +... +engine = box.space._vsession_settings:get{'sql_default_engine'}.value +--- +... +order = box.space._vsession_settings:get{'sql_reverse_unordered_selects'}.value +--- +... +box.execute('SET sql_default_engine = 1;') +--- +- null +- 'Inconsistent types: expected string got integer' +... +box.execute("SET sql_default_engine = 'some_engine';") +--- +- null +- Space engine 'some_engine' does not exist +... +box.execute("SET engine = 'vinyl';") +--- +- null +- Setting is not found +... +box.execute("SET sql_defer_foreign_keys = 'vinyl';") +--- +- null +- 'Inconsistent types: expected boolean got string' +... +engine == box.space._vsession_settings:get{'sql_default_engine'}.value +--- +- true +... +order == box.space._vsession_settings:get{'sql_reverse_unordered_selects'}.value +--- +- true +... +box.execute("SET sql_default_engine = 'vinyl';") +--- +- row_count: 0 +... +box.execute("SET sql_reverse_unordered_selects = true;") +--- +- row_count: 0 +... +box.execute('SELECT * FROM "_vsession_settings";') +--- +- metadata: + - name: name + type: string + - name: value + type: any + rows: + - ['sql_where_trace', false] + - ['sql_vdbe_trace', false] + - ['sql_vdbe_listing', false] + - ['sql_vdbe_eqp', false] + - ['sql_vdbe_debug', false] + - ['sql_vdbe_addoptrace', false] + - ['sql_trace', false] + - ['sql_select_trace', false] + - ['sql_reverse_unordered_selects', true] + - ['sql_recursive_triggers', true] + - ['sql_parser_trace', false] + - ['sql_full_column_names', false] + - ['sql_defer_foreign_keys', false] + - ['sql_default_engine', 'vinyl'] + - ['sql_compound_select_limit', 30] +... +box.execute("SET sql_default_engine = 'memtx';") +--- +- row_count: 0 +... +box.execute("SET sql_reverse_unordered_selects = false;") +--- +- row_count: 0 +... +box.execute('SELECT * FROM "_vsession_settings";') +--- +- metadata: + - name: name + type: string + - name: value + type: any + rows: + - ['sql_compound_select_limit', 30] + - ['sql_default_engine', 'memtx'] + - ['sql_defer_foreign_keys', false] + - ['sql_full_column_names', false] + - ['sql_parser_trace', false] + - ['sql_recursive_triggers', true] + - ['sql_reverse_unordered_selects', false] + - ['sql_select_trace', false] + - ['sql_trace', false] + - ['sql_vdbe_addoptrace', false] + - ['sql_vdbe_debug', false] + - ['sql_vdbe_eqp', false] + - ['sql_vdbe_listing', false] + - ['sql_vdbe_trace', false] + - ['sql_where_trace', false] +... +box.execute("SET sql_default_engine = '"..engine.."';") +--- +- row_count: 0 +... +box.execute("SET sql_reverse_unordered_selects = "..tostring(order)..";") +--- +- row_count: 0 +... diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua index edd0ef4..83746f0 100644 --- a/test/sql/sql-debug.test.lua +++ b/test/sql/sql-debug.test.lua @@ -15,3 +15,29 @@ box.execute('PRAGMA parser_trace = '.. result[1][1]) -- Make PRAGMA command return the result as a result set. -- box.execute('PRAGMA') + +-- +-- gh-4511: make sure that SET works. +-- +box.execute('SELECT "name" FROM "_vsession_settings";') + +engine = box.space._vsession_settings:get{'sql_default_engine'}.value +order = box.space._vsession_settings:get{'sql_reverse_unordered_selects'}.value + +box.execute('SET sql_default_engine = 1;') +box.execute("SET sql_default_engine = 'some_engine';") +box.execute("SET engine = 'vinyl';") +box.execute("SET sql_defer_foreign_keys = 'vinyl';") +engine == box.space._vsession_settings:get{'sql_default_engine'}.value +order == box.space._vsession_settings:get{'sql_reverse_unordered_selects'}.value + +box.execute("SET sql_default_engine = 'vinyl';") +box.execute("SET sql_reverse_unordered_selects = true;") +box.execute('SELECT * FROM "_vsession_settings";') + +box.execute("SET sql_default_engine = 'memtx';") +box.execute("SET sql_reverse_unordered_selects = false;") +box.execute('SELECT * FROM "_vsession_settings";') + +box.execute("SET sql_default_engine = '"..engine.."';") +box.execute("SET sql_reverse_unordered_selects = "..tostring(order)..";") -- 2.7.4