Re: [Tarantool-patches] [PATCH v3 3/5] sql: introduce SET statement

[Mergen Imeev <imeevma@tarantool.org>]

Hi! Thank you for review. My answers, diff and new version
below.

On Mon, Nov 11, 2019 at 10:56:46PM +0100, Vladislav Shpilevoy wrote:
> 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?
> 
All pragmas that remain after pushing this patch-set are
informational. Since they do not change any sql settings, I think
there will be no problems. I mean these pragmas: table_info,
stats, index_info, index_list, collation_list, foreign_key_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.
> 
I think this is fair, but I suggest creating an issue and moving
it to later releases.

> >>> @@ -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
Fixed. Since control pragmas will be deleted, SET is now
using VDBE I closed the issue in this commit. Still, it
may be better to close it in the last commit of the
patch-set. Should I move it to the last commit?


Diff:

From dc43af324da16f031b712b28805575e770ac4e7e Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Thu, 14 Nov 2019 20:03:45 +0300
Subject: [PATCH] Review fix


diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 322ba41..8e3ba6a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3398,17 +3398,17 @@ sql_session_opt_tuple(struct tuple_format *format, int option_id,
 	return 0;
 }
 
-static void
-sql_set_session_option(struct Parse *parse_context, int id, struct Expr *value)
+int
+sql_set_session_option(int id, void *opt_value)
 {
+	struct Expr *value = (struct Expr *)opt_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;
+		return -1;
 	}
 	if (value->type == FIELD_TYPE_BOOLEAN) {
 		bool is_set = value->op == TK_TRUE;
@@ -3429,49 +3429,53 @@ sql_set_session_option(struct Parse *parse_context, int id, struct Expr *value)
 		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;
+			return -1;
 		}
 		current_session()->sql_default_engine = engine;
 	}
+	return 0;
 }
 
-static void
-sql_set_global_option(struct Parse *parse_context, int id, struct Expr *value)
+int
+sql_set_global_option(int id, void *opt_value)
 {
+	struct Expr *value = (struct Expr *)opt_value;
 	(void)id;
 	assert(id == SQL_GLOBAL_OPTION_COMPOUND_SELECT_LIMIT);
 	int limit = value->u.iValue;
-	if (sql_limit(sql_get(), SQL_LIMIT_COMPOUND_SELECT, limit) < 0)
-		parse_context->is_aborted = true;
+	int rc = sql_limit(sql_get(), SQL_LIMIT_COMPOUND_SELECT, limit);
+	assert(rc >= 0);
+	return rc < 0 ? -1 : 0;
 }
 
 void
 sql_set_settings(struct Parse *parse_context, struct Token *name,
 		 struct Expr *value)
 {
-	int option_id;
+	int opt_id;
+	const char *val = (const char*) value;
+	struct Vdbe *vdbe = sqlGetVdbe(parse_context);
 	char *name_str = sql_name_from_token(sql_get(), name);
 	if (name_str == NULL) {
 		parse_context->is_aborted = true;
 		return;
 	}
 	/* Try to find the option among the session options. */
-	for (option_id = 0; option_id < SQL_SESSION_OPTION_max; ++option_id) {
-		if (strcasecmp(sql_session_opts[option_id].name, name_str) == 0)
-			break;
+	for (opt_id = 0; opt_id < SQL_SESSION_OPTION_max; ++opt_id) {
+		if (strcasecmp(sql_session_opts[opt_id].name, name_str) == 0) {
+			sqlVdbeAddOp4(vdbe, OP_Set, 0, opt_id, 0, val, P4_PTR);
+			return;
+		}
 	}
-	if (option_id < SQL_SESSION_OPTION_max)
-		return sql_set_session_option(parse_context, option_id, value);
 	/* Try to find the option among the global options. */
-	for (option_id = 0; option_id < SQL_GLOBAL_OPTION_max; ++option_id) {
-		if (strcasecmp(sql_global_opts[option_id].name, name_str) == 0)
-			break;
+	for (opt_id = 0; opt_id < SQL_GLOBAL_OPTION_max; ++opt_id) {
+		if (strcasecmp(sql_global_opts[opt_id].name, name_str) == 0) {
+			sqlVdbeAddOp4(vdbe, OP_Set, 1, opt_id, 0, val, P4_PTR);
+			return;
+		}
 	}
-	if (option_id < SQL_GLOBAL_OPTION_max)
-		return sql_set_global_option(parse_context, option_id, value);
 	diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Setting is not found");
 	parse_context->is_aborted = true;
 	return;
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 8a2e370..75977ce 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4468,17 +4468,38 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
 		    uint32_t *fieldno);
 
 /**
- * Set new value for SQL setting.
+ * Create VDBE instructions to set new value of 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.
+ * @param name Name of the SQL setting.
+ * @param value New value of the SQL setting.
  */
 void
 sql_set_settings(struct Parse *parse_context, struct Token *name,
 		 struct Expr *value);
 
+/**
+ * Set new value to session-local SQL setting.
+ *
+ * @param if ID of the SQL setting.
+ * @param value New value of the SQL setting.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_set_session_option(int id, void *opt_value);
+
+/**
+ * Set new value to global SQL setting.
+ *
+ * @param if ID of the SQL setting.
+ * @param value New value of the SQL setting.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_set_global_option(int id, void *opt_value);
+
 #endif				/* sqlINT_H */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index ab86be9..f45e4ff 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -5281,6 +5281,26 @@ case OP_IncMaxid: {
 	break;
 }
 
+/* Opcode: Set P1 P2 * P4 *
+ *
+ * Set new value to SQL setting. P1 is either 0 or 1. If P1 == 0,
+ * than this is session setting. If P1 == 1, than this is global
+ * setting. P2 is ID of the setting. P4 is actually of type
+ * "struct Expr *" and contains value of the setting.
+ */
+case OP_Set: {
+	assert(pOp->p1 == 0 || pOp->p1 == 1);
+	assert(pOp->p4type == P4_PTR);
+	if (pOp->p1 == 0) {
+		if (sql_set_session_option(pOp->p2, pOp->p4.p) != 0)
+			goto abort_due_to_error;
+	} else {
+		if (sql_set_global_option(pOp->p2, pOp->p4.p) != 0)
+			goto abort_due_to_error;
+	}
+	break;
+}
+
 /* Opcode: Noop * * * * *
  *
  * Do nothing.  This instruction is often useful as a jump



New patch:

From 36cede6e91074df11fb0eefdab9141cdfac0a4d5 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
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
Closes #4621

@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 <name of the setting> = <value of the setting>;

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'

All of these setting with exception of 'sql_compound_select_limit'
are session-local settings. Their value can be viewed in
_vsession_settings sysview.

Example of usage:
SET sql_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 ce87b88..8e3ba6a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3275,6 +3275,12 @@ enum {
 	SQL_SESSION_OPTION_max,
 };
 
+/**  Identifiers of all SQL global options that can be set. */
+enum {
+	SQL_GLOBAL_OPTION_COMPOUND_SELECT_LIMIT = 0,
+	SQL_GLOBAL_OPTION_max,
+};
+
 /**
  * A local structure that allows to establish a connection between
  * the name of the parameter, its field type and mask, if it have
@@ -3329,6 +3335,15 @@ static struct sql_option_metadata sql_session_opts[] = {
 #endif
 };
 
+/**
+ * Variable that contains names of the SQL global options, their
+ * field types and mask if they have one or 0 if don't have.
+ */
+static struct sql_option_metadata sql_global_opts[] = {
+	/** SQL_GLOBAL_OPTION_COMPOUND_SELECT_LIMIT */
+	{"sql_compound_select_limit", FIELD_TYPE_INTEGER, 0},
+};
+
 uint32_t
 sql_session_opt_id_max()
 {
@@ -3382,3 +3397,86 @@ sql_session_opt_tuple(struct tuple_format *format, int option_id,
 	*result = tuple;
 	return 0;
 }
+
+int
+sql_set_session_option(int id, void *opt_value)
+{
+	struct Expr *value = (struct Expr *)opt_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]);
+		return -1;
+	}
+	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) {
+			diag_set(ClientError, ER_NO_SUCH_ENGINE,
+				 value->u.zToken);
+			return -1;
+		}
+		current_session()->sql_default_engine = engine;
+	}
+	return 0;
+}
+
+int
+sql_set_global_option(int id, void *opt_value)
+{
+	struct Expr *value = (struct Expr *)opt_value;
+	(void)id;
+	assert(id == SQL_GLOBAL_OPTION_COMPOUND_SELECT_LIMIT);
+	int limit = value->u.iValue;
+	int rc = sql_limit(sql_get(), SQL_LIMIT_COMPOUND_SELECT, limit);
+	assert(rc >= 0);
+	return rc < 0 ? -1 : 0;
+}
+
+void
+sql_set_settings(struct Parse *parse_context, struct Token *name,
+		 struct Expr *value)
+{
+	int opt_id;
+	const char *val = (const char*) value;
+	struct Vdbe *vdbe = sqlGetVdbe(parse_context);
+	char *name_str = sql_name_from_token(sql_get(), name);
+	if (name_str == NULL) {
+		parse_context->is_aborted = true;
+		return;
+	}
+	/* Try to find the option among the session options. */
+	for (opt_id = 0; opt_id < SQL_SESSION_OPTION_max; ++opt_id) {
+		if (strcasecmp(sql_session_opts[opt_id].name, name_str) == 0) {
+			sqlVdbeAddOp4(vdbe, OP_Set, 0, opt_id, 0, val, P4_PTR);
+			return;
+		}
+	}
+	/* Try to find the option among the global options. */
+	for (opt_id = 0; opt_id < SQL_GLOBAL_OPTION_max; ++opt_id) {
+		if (strcasecmp(sql_global_opts[opt_id].name, name_str) == 0) {
+			sqlVdbeAddOp4(vdbe, OP_Set, 1, opt_id, 0, val, P4_PTR);
+			return;
+		}
+	}
+	diag_set(ClientError, ER_SQL_PARSER_GENERIC, "Setting is not found");
+	parse_context->is_aborted = true;
+	return;
+}
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 2594b73..75977ce 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4467,4 +4467,39 @@ int
 sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
 		    uint32_t *fieldno);
 
+/**
+ * Create VDBE instructions to set new value of SQL setting.
+ *
+ * @param parse_context Parsing context.
+ * @param name Name of the SQL setting.
+ * @param value New value of the SQL setting.
+ */
+void
+sql_set_settings(struct Parse *parse_context, struct Token *name,
+		 struct Expr *value);
+
+/**
+ * Set new value to session-local SQL setting.
+ *
+ * @param if ID of the SQL setting.
+ * @param value New value of the SQL setting.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_set_session_option(int id, void *opt_value);
+
+/**
+ * Set new value to global SQL setting.
+ *
+ * @param if ID of the SQL setting.
+ * @param value New value of the SQL setting.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_set_global_option(int id, void *opt_value);
+
 #endif				/* sqlINT_H */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index ab86be9..f45e4ff 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -5281,6 +5281,26 @@ case OP_IncMaxid: {
 	break;
 }
 
+/* Opcode: Set P1 P2 * P4 *
+ *
+ * Set new value to SQL setting. P1 is either 0 or 1. If P1 == 0,
+ * than this is session setting. If P1 == 1, than this is global
+ * setting. P2 is ID of the setting. P4 is actually of type
+ * "struct Expr *" and contains value of the setting.
+ */
+case OP_Set: {
+	assert(pOp->p1 == 0 || pOp->p1 == 1);
+	assert(pOp->p4type == P4_PTR);
+	if (pOp->p1 == 0) {
+		if (sql_set_session_option(pOp->p2, pOp->p4.p) != 0)
+			goto abort_due_to_error;
+	} else {
+		if (sql_set_global_option(pOp->p2, pOp->p4.p) != 0)
+			goto abort_due_to_error;
+	}
+	break;
+}
+
 /* Opcode: Noop * * * * *
  *
  * Do nothing.  This instruction is often useful as a jump
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index b190753..6276bcc 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -54,3 +54,131 @@ 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_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']
+...
+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_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)..";")