From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 423DC452566 for ; Tue, 12 Nov 2019 00:50:41 +0300 (MSK) References: <12ed4be2e7e433fdca58a43fc3b937eb9a54f52f.1573121685.git.imeevma@gmail.com> <9fe3bd05-17de-e878-4395-4d15cf2f0b38@tarantool.org> <20191107141209.GA10466@tarantool.org> From: Vladislav Shpilevoy Message-ID: <93979a0b-1708-1125-1142-74f22734b088@tarantool.org> Date: Mon, 11 Nov 2019 22:56:46 +0100 MIME-Version: 1.0 In-Reply-To: <20191107141209.GA10466@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v3 3/5] sql: introduce SET statement List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mergen Imeev Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the fixes! See 3 comments below. >>> SET sql_compound_select_limit = 10; >>> SET sql_default_engine = 'memtx'; >> >> 2. It would be cool to sum up here what is a purpose of >> PRAGMA and SET. How are they different. From the code I >> see, that looks like PRAGMA never changes anything. It >> only returns something. While SET only sets and never >> returns, right? >> > How pragma works: > PRAGMA vdbe_trace = true; -- Sets vdbe_trace, returns nothing. > PRAGMA vdbe_trace; -- Returns current value of vdbe_trace flag. 1. These PRAGMAs don't exist anymore. I wanted to understand how rest of the pragmas (not removed) coexist with SET. When a user should try to look at PRAGMA list, and when at SET list? > > How SET works: > SET sql_vdbe_trace = true; -- Sets vdbe_trace, returns nothing. > >> In the next patch I see diff like this: >> >>> - return test:execsql "PRAGMA full_column_names" >>> - end, { >>> + return box.space._vsession_settings:get("sql_full_column_names").value >>> + end, >> >> Is there any plan how to make it simpler? Seems like it is >> impossible for a user to get session settings other than via >> direct select from a system space. It was simpler with PRAGMA. >> > As far as I know, there is no such plans. It definitely was > simpler with PRAGMA, but it was decided to remove PRAGMA. > I do not know exact reaso why. > 2. Ok, but now it looks really unusable when a user want's to learn an option value. I will create a ticket, if this patchset will be pushed and nobody will care about usability beforehand. >>> @@ -3382,3 +3408,82 @@ sql_session_opt_tuple(struct tuple_format *format, int option_id, >>> *result = tuple; >>> return 0; >>> } >>> + >>> +static void >>> +sql_set_session_option(struct Parse *parse_context, int id, struct Expr *value) >>> +{ >>> + struct session *session = current_session(); >>> + struct sql_option_metadata *option = &sql_session_opts[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 (id == SQL_SESSION_OPTION_PARSER_TRACE) { >>> + if (is_set) >>> + sqlParserTrace(stdout, "parser: "); >>> + else >>> + sqlParserTrace(NULL, NULL); >>> + } >>> +#endif >>> + } else { >>> + assert(id == SQL_SESSION_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; >>> + } >>> +} >> >> 5. Wait. So the settings are set during parsing? Isn't it wrong? >> 'Parser only parses' and everything? PRAGMA works in runtime. > No, PRAGMA also set during parsing. I once suggested to > change it, so parser would only parse, but my suggestion > was declined. 3. This is bad and should be fixed. https://github.com/tarantool/tarantool/issues/4621