On 17 Mar 20:26, Chris Sosnin wrote:
On 16 Mar 2020, at 20:02, Nikita Pettik <korablev@tarantool.org> wrote:
On 17 Feb 15:12, Chris Sosnin wrote:
Currently if a user wants to change session setting with sql, he has
to execute non-obvious query, thus, we introduce a more native way to
do this.
It is not about non-obvious queries, but it is all about documentation:
the better feature is described, the clearer its usage turns out to be
for user.
Should I change the commit message? I though this patchset is about simplifying
the way session settings are updated?
I would say:Currently if a user wants to change session settings via SQL, one hasto execute UPDATE query like:[[UPDATE "_session_settings" SET "value" = true WHERE "name" = 'name']]However, direct access to system spaces isn't considered to be good practice.To avoid that and a bit simplify user's life, we introduce SQL shortcut commandSET with following syntax: SET <setting_name> = <value>. <setting_name> issupposed to be name of setting (note that it is uppercased as any othernon-quoted indentifiers in SQL) and <value> - value of corresponding setting tobe set; <value> should be either literal or binding marker. Also it is worthnoting that SET doesn't provide any implicit casts, so <value> must be of thetype corresponding to the setting being updated. Example:...^ It is up to you.
I took the beginning as long as the rest is described in the doc request (except for
implicit casts note, thanks for pointing that out, fixed):
Currently if a user wants to change session setting with SQL, one has
to execute UPDATE query like:
[[UPDATE "_session_settings" SET "value" = true WHERE "name" = 'name']]
However, direct access to system spaces isn't considered to be a good practice.
To avoid that and a bit simplify user's life, we introduce SQL shortcut command
SETTING SET.
Closes #4711
@TarantoolBot document
Title: API for accessing _session_settings space.
There are two ways of updating values of session settings:
via Lua and SQL.
SQL:
Instead of typing long UPDATE query one can use the SET statement:
`box.execute([[SET "<setting_name>" = <new_value>]])`.
Note, that this query is case sensitive so the name must be quoted.
Example:
```
tarantool> box.execute([[set "sql_default_engine" = 'memtx']])
---
- row_count: 1
...
tarantool> box.execute([[set "sql_defer_foreign_keys" = true]])
Why didn't you consider show/get command to retrieve setting value?
Otherwise, you simplify way to set session local options, but doesn't
provide the same way to extract current values.
SELECT * FROM “_session_settings” is simple enough, isn’t it?
In this case you'll get list of all settings. To get specific onewe should use this query:SELECT value FROM "_session_settings" WHERE "name" = 'xxx'I do not insist on implementing GET/SHOW SQL syntax now tho.
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index d1fcf4761..3ffae5970 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4510,4 +4510,15 @@ int
sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
uint32_t *fieldno);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 620d74e66..c81486fa6 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -55,6 +55,7 @@
#include "box/schema.h"
#include "box/space.h"
#include "box/sequence.h"
+#include "box/session_settings.h"
/*
* Invoke this macro on memory cells just prior to changing the
@@ -5248,6 +5249,55 @@ case OP_IncMaxid: {
break;
}
+/* Opcode: Set P1 P2 * * *
+ *
+ * Set the new value of the session setting. P1 is the id of the
+ * setting in the session_settings array, P2 is the register
+ * holding a value.
+ */
+case OP_Set: {
Please, use more specific opcode name. Like OP_SettingSet.
Didn’t we accept ’set’ syntax?
SET is okay, but I don't mind SET SETTING as well. OP_Set is too generalname - SET keyword is also part of UPDATE syntax, so it may confuse otherdevelopers. Except for this nit patch LGTM.
Here are the fixes:
- sqlVdbeAddOp2(vdbe, OP_Set, index, target);
+ sqlVdbeAddOp2(vdbe, OP_SetSetting, index, target);
-///////////////////////////// The SET command ////////////////////////////////
-cmd ::= SET nm(X) EQ expr(Y). {
+///////////////////////////// The SETTING SET command ////////////////////////
+cmd ::= SETTING SET nm(X) EQ expr(Y). {
-/* Opcode: Set P1 P2 * * *
+/* Opcode: SetSetting P1 P2 * * *
-case OP_Set: {
+case OP_SetSetting: {
And of course I had to change the grammar and tests.
Changes are on the branch with the second version of Lua patch: