* [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space
@ 2020-07-08 11:47 Roman Khabibov
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages Roman Khabibov
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Roman Khabibov @ 2020-07-08 11:47 UTC (permalink / raw)
To: tarantool-patches
@ChangeLog
- Fix _session_settings name in error messages
- Raname "sql_defer_foreign_keys" setting to
"sql_defer_foreign_key_changes".
Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4732-ss-space
Issue: https://github.com/tarantool/tarantool/issues/4732
Roman Khabibov (2):
sql: fix _session_settings name in error messages
sql: clarify "sql_defer_foreign_keys" setting name
src/box/session_settings.c | 10 +-
test/box/session_settings.result | 108 +++++++++++-----------
test/box/session_settings.test.lua | 86 ++++++++---------
test/sql/transitive-transactions.result | 4 +-
test/sql/transitive-transactions.test.lua | 4 +-
5 files changed, 106 insertions(+), 106 deletions(-)
--
2.21.0 (Apple Git-122)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages
2020-07-08 11:47 [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space Roman Khabibov
@ 2020-07-08 11:47 ` Roman Khabibov
2020-07-14 16:07 ` Mergen Imeev
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name Roman Khabibov
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Roman Khabibov @ 2020-07-08 11:47 UTC (permalink / raw)
To: tarantool-patches
Print the true name of _session_settings space in error messages.
Follow up #4511
---
src/box/session_settings.c | 8 ++++----
test/box/session_settings.result | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/box/session_settings.c b/src/box/session_settings.c
index dbbbf2461..2f4b8c2f0 100644
--- a/src/box/session_settings.c
+++ b/src/box/session_settings.c
@@ -301,7 +301,7 @@ session_settings_space_execute_replace(struct space *space, struct txn *txn,
(void)txn;
(void)request;
(void)result;
- diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
"replace()");
return -1;
}
@@ -315,7 +315,7 @@ session_settings_space_execute_delete(struct space *space, struct txn *txn,
(void)txn;
(void)request;
(void)result;
- diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
"delete()");
return -1;
}
@@ -388,7 +388,7 @@ session_settings_space_execute_upsert(struct space *space, struct txn *txn,
(void)space;
(void)txn;
(void)request;
- diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
"upsert()");
return -1;
}
@@ -398,7 +398,7 @@ session_settings_space_create_index(struct space *space, struct index_def *def)
{
assert(space->def->id == BOX_SESSION_SETTINGS_ID);
if (def->iid != 0) {
- diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
"create_index()");
return NULL;
}
diff --git a/test/box/session_settings.result b/test/box/session_settings.result
index 149cc4bd5..d38707ef8 100644
--- a/test/box/session_settings.result
+++ b/test/box/session_settings.result
@@ -30,19 +30,19 @@ s:drop()
--
s:create_index('a')
| ---
- | - error: Session_settings space does not support create_index()
+ | - error: _session_settings space does not support create_index()
| ...
s:insert({'a', 1})
| ---
- | - error: Session_settings space does not support replace()
+ | - error: _session_settings space does not support replace()
| ...
s:delete({'b'})
| ---
- | - error: Session_settings space does not support delete()
+ | - error: _session_settings space does not support delete()
| ...
s:replace({'sql_defer_foreign_keys', true})
| ---
- | - error: Session_settings space does not support replace()
+ | - error: _session_settings space does not support replace()
| ...
--
--
2.21.0 (Apple Git-122)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name
2020-07-08 11:47 [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space Roman Khabibov
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages Roman Khabibov
@ 2020-07-08 11:47 ` Roman Khabibov
2020-07-12 15:31 ` Vladislav Shpilevoy
2020-07-14 16:22 ` Mergen Imeev
2020-07-14 16:01 ` [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space Mergen Imeev
2020-11-26 9:51 ` Kirill Yukhin
3 siblings, 2 replies; 12+ messages in thread
From: Roman Khabibov @ 2020-07-08 11:47 UTC (permalink / raw)
To: tarantool-patches
Raname "sql_defer_foreign_keys" setting to
"sql_defer_foreign_key_changes".
Follow up #4511
@TarantoolBot document
Title: sql_defer_foreign_keys in _space_settings
Rename "sql_defer_foreign_keys" setting to
"sql_defer_foreign_key_changes".
---
src/box/session_settings.c | 2 +-
test/box/session_settings.result | 100 +++++++++++-----------
test/box/session_settings.test.lua | 86 +++++++++----------
test/sql/transitive-transactions.result | 4 +-
test/sql/transitive-transactions.test.lua | 4 +-
5 files changed, 98 insertions(+), 98 deletions(-)
diff --git a/src/box/session_settings.c b/src/box/session_settings.c
index 2f4b8c2f0..01cb91ade 100644
--- a/src/box/session_settings.c
+++ b/src/box/session_settings.c
@@ -44,7 +44,7 @@ struct session_setting session_settings[SESSION_SETTING_COUNT] = {};
const char *session_setting_strs[SESSION_SETTING_COUNT] = {
"error_marshaling_enabled",
"sql_default_engine",
- "sql_defer_foreign_keys",
+ "sql_defer_foreign_key_changes",
"sql_full_column_names",
"sql_full_metadata",
"sql_parser_debug",
diff --git a/test/box/session_settings.result b/test/box/session_settings.result
index d38707ef8..cf94da48c 100644
--- a/test/box/session_settings.result
+++ b/test/box/session_settings.result
@@ -40,7 +40,7 @@ s:delete({'b'})
| ---
| - error: _session_settings space does not support delete()
| ...
-s:replace({'sql_defer_foreign_keys', true})
+s:replace({'sql_defer_foreign_key_changes', true})
| ---
| - error: _session_settings space does not support replace()
| ...
@@ -54,7 +54,7 @@ s:select()
| ---
| - - ['error_marshaling_enabled', false]
| - ['sql_default_engine', 'memtx']
- | - ['sql_defer_foreign_keys', false]
+ | - ['sql_defer_foreign_key_changes', false]
| - ['sql_full_column_names', false]
| - ['sql_full_metadata', false]
| - ['sql_parser_debug', false]
@@ -112,7 +112,7 @@ check_sorting(s, t, 'sql_d')
check_sorting(s, t, 'sql_v')
| ---
| ...
-check_sorting(s, t, 'sql_defer_foreign_keys')
+check_sorting(s, t, 'sql_defer_foreign_key_changes')
| ---
| ...
@@ -121,9 +121,9 @@ t:drop()
| ...
-- Check get() method of session_settings space.
-s:get({'sql_defer_foreign_keys'})
+s:get({'sql_defer_foreign_key_changes'})
| ---
- | - ['sql_defer_foreign_keys', false]
+ | - ['sql_defer_foreign_key_changes', false]
| ...
s:get({'sql_recursive_triggers'})
| ---
@@ -156,13 +156,13 @@ for key, value in s:pairs() do table.insert(t, {key, value}) end
-- Check update() method of session_settings space.
-- Correct updates.
-s:update('sql_defer_foreign_keys', {{'=', 'value', true}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', true}})
| ---
- | - ['sql_defer_foreign_keys', true]
+ | - ['sql_defer_foreign_key_changes', true]
| ...
-s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}})
+s:update({'sql_defer_foreign_key_changes'}, {{'=', 2, false}})
| ---
- | - ['sql_defer_foreign_keys', false]
+ | - ['sql_defer_foreign_key_changes', false]
| ...
s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
| ---
@@ -177,116 +177,116 @@ s:update('a', {{'=', 2, 1}})
| ...
-- Inorrect updates.
-s:update({{'sql_defer_foreign_keys'}}, {{'=', 'value', true}})
+s:update({{'sql_defer_foreign_key_changes'}}, {{'=', 'value', true}})
| ---
| - error: 'Supplied key type of part 0 does not match index part type: expected string'
| ...
-s:update('sql_defer_foreign_keys', {'=', 'value', true})
+s:update('sql_defer_foreign_key_changes', {'=', 'value', true})
| ---
| - error: Illegal parameters, update operation must be an array {op,..}
| ...
-s:update('sql_defer_foreign_keys', {{'=', 'value', true}, {'=', 2, true}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', true}, {'=', 2, true}})
| ---
- | - ['sql_defer_foreign_keys', true]
+ | - ['sql_defer_foreign_key_changes', true]
| ...
-s:update('sql_defer_foreign_keys', {{}})
+s:update('sql_defer_foreign_key_changes', {{}})
| ---
| - error: Illegal parameters, update operation must be an array {op,..}, got empty
| array
| ...
-s:update('sql_defer_foreign_keys', {{'='}})
+s:update('sql_defer_foreign_key_changes', {{'='}})
| ---
| - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
| 1'
| ...
-s:update('sql_defer_foreign_keys', {{'=', 'value'}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value'}})
| ---
| - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
| 2'
| ...
-s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', true, 1}})
| ---
| - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
| 4'
| ...
-s:update('sql_defer_foreign_keys', {{'+', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'+', 'value', 2}})
| ---
| - error: 'Argument type in operation ''+'' on field ''value'' does not match field
| type: expected a number'
| ...
-s:update('sql_defer_foreign_keys', {{'-', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'-', 'value', 2}})
| ---
| - error: 'Argument type in operation ''-'' on field ''value'' does not match field
| type: expected a number'
| ...
-s:update('sql_defer_foreign_keys', {{'&', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'&', 'value', 2}})
| ---
| - error: 'Argument type in operation ''&'' on field ''value'' does not match field
| type: expected a positive integer'
| ...
-s:update('sql_defer_foreign_keys', {{'|', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'|', 'value', 2}})
| ---
| - error: 'Argument type in operation ''|'' on field ''value'' does not match field
| type: expected a positive integer'
| ...
-s:update('sql_defer_foreign_keys', {{'^', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'^', 'value', 2}})
| ---
| - error: 'Argument type in operation ''^'' on field ''value'' does not match field
| type: expected a positive integer'
| ...
-s:update('sql_defer_foreign_keys', {{'!', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'!', 'value', 2}})
| ---
| - error: Tuple field count 3 does not match space field count 2
| ...
-s:update('sql_defer_foreign_keys', {{'#', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'#', 'value', 2}})
| ---
| - error: Tuple field count 1 does not match space field count 2
| ...
-s:update('sql_defer_foreign_keys', {{1, 'value', true}})
+s:update('sql_defer_foreign_key_changes', {{1, 'value', true}})
| ---
| - error: Illegal parameters, update operation name must be a string
| ...
-s:update('sql_defer_foreign_keys', {{{1}, 'value', true}})
+s:update('sql_defer_foreign_key_changes', {{{1}, 'value', true}})
| ---
| - error: Illegal parameters, update operation name must be a string
| ...
-s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}})
+s:update('sql_defer_foreign_key_changes', {{'=', {'value'}, true}})
| ---
| - error: Illegal parameters, field id must be a number or a string
| ...
-s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}})
+s:update('sql_defer_foreign_key_changes', {{'=', 1, 'new_key'}})
| ---
| - error: Attempt to modify a tuple field which is part of index 'primary' in space
| '_session_settings'
| ...
-s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'name', 'new_key'}})
| ---
| - error: Attempt to modify a tuple field which is part of index 'primary' in space
| '_session_settings'
| ...
-s:update('sql_defer_foreign_keys', {{'=', 3, true}})
+s:update('sql_defer_foreign_key_changes', {{'=', 3, true}})
| ---
| - error: Tuple field count 3 does not match space field count 2
| ...
-s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'some text', true}})
| ---
| - error: Field 'some text' was not found in the tuple
| ...
-s:update('sql_defer_foreign_keys', {{'=', 'value', 1}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', 1}})
| ---
- | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
+ | - error: Session setting sql_defer_foreign_key_changes expected a value of type boolean
| ...
-s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', {1}}})
| ---
- | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
+ | - error: Session setting sql_defer_foreign_key_changes expected a value of type boolean
| ...
-s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', '1'}})
| ---
- | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
+ | - error: Session setting sql_defer_foreign_key_changes expected a value of type boolean
| ...
-- gh-4711: Provide a user-friendly frontend for accessing session settings.
@@ -313,18 +313,18 @@ s:get('sql_default_engine').value
| ---
| - memtx
| ...
-settings.sql_defer_foreign_keys = true
+settings.sql_defer_foreign_key_changes = true
| ---
| ...
-s:get('sql_defer_foreign_keys').value
+s:get('sql_defer_foreign_key_changes').value
| ---
| - true
| ...
-s:update('sql_defer_foreign_keys', {{'=', 2, false}})
+s:update('sql_defer_foreign_key_changes', {{'=', 2, false}})
| ---
- | - ['sql_defer_foreign_keys', false]
+ | - ['sql_defer_foreign_key_changes', false]
| ...
-settings.sql_defer_foreign_keys
+settings.sql_defer_foreign_key_changes
| ---
| - false
| ...
@@ -345,19 +345,19 @@ s:get('sql_default_engine').value
| ---
| - memtx
| ...
-box.execute([[set session "sql_defer_foreign_keys" = true]])
+box.execute([[set session "sql_defer_foreign_key_changes" = true]])
| ---
| - row_count: 1
| ...
-s:get('sql_defer_foreign_keys').value
+s:get('sql_defer_foreign_key_changes').value
| ---
| - true
| ...
-box.execute([[set session "sql_defer_foreign_keys" = false]])
+box.execute([[set session "sql_defer_foreign_key_changes" = false]])
| ---
| - row_count: 1
| ...
-s:get('sql_defer_foreign_keys').value
+s:get('sql_defer_foreign_key_changes').value
| ---
| - false
| ...
@@ -366,9 +366,9 @@ settings.sql_default_engine = true
| ---
| - error: Session setting sql_default_engine expected a value of type string
| ...
-settings.sql_defer_foreign_keys = 'false'
+settings.sql_defer_foreign_key_changes = 'false'
| ---
- | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
+ | - error: Session setting sql_defer_foreign_key_changes expected a value of type boolean
| ...
settings.sql_parser_debug = 'string'
| ---
@@ -393,8 +393,8 @@ box.execute([[set session "sql_default_engine" = true]])
| - null
| - Session setting sql_default_engine expected a value of type string
| ...
-box.execute([[set session "sql_defer_foreign_keys" = 'true']])
+box.execute([[set session "sql_defer_foreign_key_changes" = 'true']])
| ---
| - null
- | - Session setting sql_defer_foreign_keys expected a value of type boolean
+ | - Session setting sql_defer_foreign_key_changes expected a value of type boolean
| ...
diff --git a/test/box/session_settings.test.lua b/test/box/session_settings.test.lua
index 70c1e00be..468b04d56 100644
--- a/test/box/session_settings.test.lua
+++ b/test/box/session_settings.test.lua
@@ -17,7 +17,7 @@ s:drop()
s:create_index('a')
s:insert({'a', 1})
s:delete({'b'})
-s:replace({'sql_defer_foreign_keys', true})
+s:replace({'sql_defer_foreign_key_changes', true})
--
-- Check select() method of session_settings space. Should work
@@ -52,12 +52,12 @@ check_sorting(s, t)
check_sorting(s, t, 'abcde')
check_sorting(s, t, 'sql_d')
check_sorting(s, t, 'sql_v')
-check_sorting(s, t, 'sql_defer_foreign_keys')
+check_sorting(s, t, 'sql_defer_foreign_key_changes')
t:drop()
-- Check get() method of session_settings space.
-s:get({'sql_defer_foreign_keys'})
+s:get({'sql_defer_foreign_key_changes'})
s:get({'sql_recursive_triggers'})
s:get({'sql_reverse_unordered_selects'})
s:get({'sql_default_engine'})
@@ -71,41 +71,41 @@ for key, value in s:pairs() do table.insert(t, {key, value}) end
-- Check update() method of session_settings space.
-- Correct updates.
-s:update('sql_defer_foreign_keys', {{'=', 'value', true}})
-s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', true}})
+s:update({'sql_defer_foreign_key_changes'}, {{'=', 2, false}})
s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
s:update('sql_default_engine', {{':', 'value', 1, 5, 'memtx'}})
s:update('a', {{'=', 2, 1}})
-- Inorrect updates.
-s:update({{'sql_defer_foreign_keys'}}, {{'=', 'value', true}})
-
-s:update('sql_defer_foreign_keys', {'=', 'value', true})
-s:update('sql_defer_foreign_keys', {{'=', 'value', true}, {'=', 2, true}})
-s:update('sql_defer_foreign_keys', {{}})
-s:update('sql_defer_foreign_keys', {{'='}})
-s:update('sql_defer_foreign_keys', {{'=', 'value'}})
-s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}})
-
-s:update('sql_defer_foreign_keys', {{'+', 'value', 2}})
-s:update('sql_defer_foreign_keys', {{'-', 'value', 2}})
-s:update('sql_defer_foreign_keys', {{'&', 'value', 2}})
-s:update('sql_defer_foreign_keys', {{'|', 'value', 2}})
-s:update('sql_defer_foreign_keys', {{'^', 'value', 2}})
-s:update('sql_defer_foreign_keys', {{'!', 'value', 2}})
-s:update('sql_defer_foreign_keys', {{'#', 'value', 2}})
-s:update('sql_defer_foreign_keys', {{1, 'value', true}})
-s:update('sql_defer_foreign_keys', {{{1}, 'value', true}})
-
-s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}})
-s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}})
-s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}})
-s:update('sql_defer_foreign_keys', {{'=', 3, true}})
-s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
-
-s:update('sql_defer_foreign_keys', {{'=', 'value', 1}})
-s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
-s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
+s:update({{'sql_defer_foreign_key_changes'}}, {{'=', 'value', true}})
+
+s:update('sql_defer_foreign_key_changes', {'=', 'value', true})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', true}, {'=', 2, true}})
+s:update('sql_defer_foreign_key_changes', {{}})
+s:update('sql_defer_foreign_key_changes', {{'='}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value'}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', true, 1}})
+
+s:update('sql_defer_foreign_key_changes', {{'+', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'-', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'&', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'|', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'^', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'!', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{'#', 'value', 2}})
+s:update('sql_defer_foreign_key_changes', {{1, 'value', true}})
+s:update('sql_defer_foreign_key_changes', {{{1}, 'value', true}})
+
+s:update('sql_defer_foreign_key_changes', {{'=', {'value'}, true}})
+s:update('sql_defer_foreign_key_changes', {{'=', 1, 'new_key'}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'name', 'new_key'}})
+s:update('sql_defer_foreign_key_changes', {{'=', 3, true}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'some text', true}})
+
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', 1}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', {1}}})
+s:update('sql_defer_foreign_key_changes', {{'=', 'value', '1'}})
-- gh-4711: Provide a user-friendly frontend for accessing session settings.
settings = box.session.settings
@@ -115,22 +115,22 @@ s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
settings.sql_default_engine
settings.sql_default_engine = 'memtx'
s:get('sql_default_engine').value
-settings.sql_defer_foreign_keys = true
-s:get('sql_defer_foreign_keys').value
-s:update('sql_defer_foreign_keys', {{'=', 2, false}})
-settings.sql_defer_foreign_keys
+settings.sql_defer_foreign_key_changes = true
+s:get('sql_defer_foreign_key_changes').value
+s:update('sql_defer_foreign_key_changes', {{'=', 2, false}})
+settings.sql_defer_foreign_key_changes
box.execute([[set session "sql_default_engine" = 'vinyl']])
s:get('sql_default_engine').value
box.execute([[set session "sql_default_engine" = 'memtx']])
s:get('sql_default_engine').value
-box.execute([[set session "sql_defer_foreign_keys" = true]])
-s:get('sql_defer_foreign_keys').value
-box.execute([[set session "sql_defer_foreign_keys" = false]])
-s:get('sql_defer_foreign_keys').value
+box.execute([[set session "sql_defer_foreign_key_changes" = true]])
+s:get('sql_defer_foreign_key_changes').value
+box.execute([[set session "sql_defer_foreign_key_changes" = false]])
+s:get('sql_defer_foreign_key_changes').value
settings.sql_default_engine = true
-settings.sql_defer_foreign_keys = 'false'
+settings.sql_defer_foreign_key_changes = 'false'
settings.sql_parser_debug = 'string'
str = string.rep('a', 20 * 1024)
@@ -138,4 +138,4 @@ box.session.settings.sql_default_engine = str
box.execute([[set session "sql_def_engine" = true]])
box.execute([[set session "sql_default_engine" = true]])
-box.execute([[set session "sql_defer_foreign_keys" = 'true']])
+box.execute([[set session "sql_defer_foreign_key_changes" = 'true']])
diff --git a/test/sql/transitive-transactions.result b/test/sql/transitive-transactions.result
index 91c35a309..c65c123ea 100644
--- a/test/sql/transitive-transactions.result
+++ b/test/sql/transitive-transactions.result
@@ -115,7 +115,7 @@ box.space.PARENT:select();
---
- - [1, 1]
...
-box.space._session_settings:update('sql_defer_foreign_keys', {{'=', 2, true}})
+box.space._session_settings:update('sql_defer_foreign_key_changes', {{'=', 2, true}})
box.rollback()
fk_defer();
---
@@ -130,7 +130,7 @@ box.space.PARENT:select();
- [2, 2]
...
-- Cleanup
-box.space._session_settings:update('sql_defer_foreign_keys', {{'=', 2, false}})
+box.space._session_settings:update('sql_defer_foreign_key_changes', {{'=', 2, false}})
box.execute('DROP TABLE child;');
---
diff --git a/test/sql/transitive-transactions.test.lua b/test/sql/transitive-transactions.test.lua
index 5565de7fe..e9e7bab92 100644
--- a/test/sql/transitive-transactions.test.lua
+++ b/test/sql/transitive-transactions.test.lua
@@ -62,12 +62,12 @@ end;
fk_defer();
box.space.CHILD:select();
box.space.PARENT:select();
-box.space._session_settings:update('sql_defer_foreign_keys', {{'=', 2, true}})
+box.space._session_settings:update('sql_defer_foreign_key_changes', {{'=', 2, true}})
box.rollback()
fk_defer();
box.space.CHILD:select();
box.space.PARENT:select();
-box.space._session_settings:update('sql_defer_foreign_keys', {{'=', 2, false}})
+box.space._session_settings:update('sql_defer_foreign_key_changes', {{'=', 2, false}})
-- Cleanup
box.execute('DROP TABLE child;');
--
2.21.0 (Apple Git-122)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name Roman Khabibov
@ 2020-07-12 15:31 ` Vladislav Shpilevoy
2020-07-12 22:11 ` Nikita Pettik
2020-07-14 16:22 ` Mergen Imeev
1 sibling, 1 reply; 12+ messages in thread
From: Vladislav Shpilevoy @ 2020-07-12 15:31 UTC (permalink / raw)
To: Roman Khabibov, tarantool-patches
Hi! Thanks for the patch!
See 3 comments below.
On 08.07.2020 13:47, Roman Khabibov wrote:
> Raname "sql_defer_foreign_keys" setting to
> "sql_defer_foreign_key_changes".
1. 'Raname' -> 'Rename'.
2. I think the new name is worse. Changes are done immediately.
You can't defer them. The flag means defer the foreign key checks,
not changes. To allow temporary inconsistency if a transaction needs
it, but still check FKs when it will commit.
> Follow up #4511
>
> @TarantoolBot document
> Title: sql_defer_foreign_keys in _space_settings
3. There is no such space '_space_settings'.
> Rename "sql_defer_foreign_keys" setting to
> "sql_defer_foreign_key_changes".
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name
2020-07-12 15:31 ` Vladislav Shpilevoy
@ 2020-07-12 22:11 ` Nikita Pettik
0 siblings, 0 replies; 12+ messages in thread
From: Nikita Pettik @ 2020-07-12 22:11 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
On 12 Jul 17:31, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
> See 3 comments below.
>
> On 08.07.2020 13:47, Roman Khabibov wrote:
> > Raname "sql_defer_foreign_keys" setting to
> > "sql_defer_foreign_key_changes".
>
> 1. 'Raname' -> 'Rename'.
>
> 2. I think the new name is worse. Changes are done immediately.
> You can't defer them. The flag means defer the foreign key checks,
> not changes. To allow temporary inconsistency if a transaction needs
> it, but still check FKs when it will commit.
Agree, this renaming looks pointless and even misleading.
I see it was suggested by Peter, but I don't see enough
arguments to change the name. Please, reject the patch.
> > Follow up #4511
> >
> > @TarantoolBot document
> > Title: sql_defer_foreign_keys in _space_settings
>
> 3. There is no such space '_space_settings'.
>
> > Rename "sql_defer_foreign_keys" setting to
> > "sql_defer_foreign_key_changes".
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space
2020-07-08 11:47 [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space Roman Khabibov
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages Roman Khabibov
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name Roman Khabibov
@ 2020-07-14 16:01 ` Mergen Imeev
2020-11-26 9:51 ` Kirill Yukhin
3 siblings, 0 replies; 12+ messages in thread
From: Mergen Imeev @ 2020-07-14 16:01 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
Hi! Thank you for the patches. See 2 comments below.
On Wed, Jul 08, 2020 at 02:47:16PM +0300, Roman Khabibov wrote:
1. Please write some description of the patch-set.
> @ChangeLog
> - Fix _session_settings name in error messages
> - Raname "sql_defer_foreign_keys" setting to
> "sql_defer_foreign_key_changes".
2. Please refer to the 'Code review procedure' on the Wiki for the
rules for writing ChangeLog correctly. It should be written on one
line and with the issue number.
>
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4732-ss-space
> Issue: https://github.com/tarantool/tarantool/issues/4732
>
> Roman Khabibov (2):
> sql: fix _session_settings name in error messages
> sql: clarify "sql_defer_foreign_keys" setting name
>
> src/box/session_settings.c | 10 +-
> test/box/session_settings.result | 108 +++++++++++-----------
> test/box/session_settings.test.lua | 86 ++++++++---------
> test/sql/transitive-transactions.result | 4 +-
> test/sql/transitive-transactions.test.lua | 4 +-
> 5 files changed, 106 insertions(+), 106 deletions(-)
>
> --
> 2.21.0 (Apple Git-122)
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages Roman Khabibov
@ 2020-07-14 16:07 ` Mergen Imeev
2020-10-08 16:17 ` Roman Khabibov
0 siblings, 1 reply; 12+ messages in thread
From: Mergen Imeev @ 2020-07-14 16:07 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
See 1 comment below.
On Wed, Jul 08, 2020 at 02:47:17PM +0300, Roman Khabibov wrote:
> Print the true name of _session_settings space in error messages.
>
> Follow up #4511
1. Please write the actual issue number here.
> ---
> src/box/session_settings.c | 8 ++++----
> test/box/session_settings.result | 8 ++++----
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
> index dbbbf2461..2f4b8c2f0 100644
> --- a/src/box/session_settings.c
> +++ b/src/box/session_settings.c
> @@ -301,7 +301,7 @@ session_settings_space_execute_replace(struct space *space, struct txn *txn,
> (void)txn;
> (void)request;
> (void)result;
> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> "replace()");
> return -1;
> }
> @@ -315,7 +315,7 @@ session_settings_space_execute_delete(struct space *space, struct txn *txn,
> (void)txn;
> (void)request;
> (void)result;
> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> "delete()");
> return -1;
> }
> @@ -388,7 +388,7 @@ session_settings_space_execute_upsert(struct space *space, struct txn *txn,
> (void)space;
> (void)txn;
> (void)request;
> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> "upsert()");
> return -1;
> }
> @@ -398,7 +398,7 @@ session_settings_space_create_index(struct space *space, struct index_def *def)
> {
> assert(space->def->id == BOX_SESSION_SETTINGS_ID);
> if (def->iid != 0) {
> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> "create_index()");
> return NULL;
> }
> diff --git a/test/box/session_settings.result b/test/box/session_settings.result
> index 149cc4bd5..d38707ef8 100644
> --- a/test/box/session_settings.result
> +++ b/test/box/session_settings.result
> @@ -30,19 +30,19 @@ s:drop()
> --
> s:create_index('a')
> | ---
> - | - error: Session_settings space does not support create_index()
> + | - error: _session_settings space does not support create_index()
> | ...
> s:insert({'a', 1})
> | ---
> - | - error: Session_settings space does not support replace()
> + | - error: _session_settings space does not support replace()
> | ...
> s:delete({'b'})
> | ---
> - | - error: Session_settings space does not support delete()
> + | - error: _session_settings space does not support delete()
> | ...
> s:replace({'sql_defer_foreign_keys', true})
> | ---
> - | - error: Session_settings space does not support replace()
> + | - error: _session_settings space does not support replace()
> | ...
>
> --
> --
> 2.21.0 (Apple Git-122)
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name Roman Khabibov
2020-07-12 15:31 ` Vladislav Shpilevoy
@ 2020-07-14 16:22 ` Mergen Imeev
1 sibling, 0 replies; 12+ messages in thread
From: Mergen Imeev @ 2020-07-14 16:22 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
See 4 comments below.
On Wed, Jul 08, 2020 at 02:47:18PM +0300, Roman Khabibov wrote:
> Raname "sql_defer_foreign_keys" setting to
> "sql_defer_foreign_key_changes".
>
> Follow up #4511
1. Please write an actual issue number here.
>
> @TarantoolBot document
> Title: sql_defer_foreign_keys in _space_settings
>
> Rename "sql_defer_foreign_keys" setting to
> "sql_defer_foreign_key_changes".
2. Please write the proper doc-bot request. It should include a
name, description and examples. Use the correct formatting, as
your request will be used to create an issue.
> ---
> src/box/session_settings.c | 2 +-
> test/box/session_settings.result | 100 +++++++++++-----------
> test/box/session_settings.test.lua | 86 +++++++++----------
> test/sql/transitive-transactions.result | 4 +-
> test/sql/transitive-transactions.test.lua | 4 +-
> 5 files changed, 98 insertions(+), 98 deletions(-)
>
> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
> index 2f4b8c2f0..01cb91ade 100644
> --- a/src/box/session_settings.c
> +++ b/src/box/session_settings.c
> @@ -44,7 +44,7 @@ struct session_setting session_settings[SESSION_SETTING_COUNT] = {};
> const char *session_setting_strs[SESSION_SETTING_COUNT] = {
> "error_marshaling_enabled",
> "sql_default_engine",
> - "sql_defer_foreign_keys",
> + "sql_defer_foreign_key_changes",
3. Why did you choose this name? This is different from the one
in issue.
> "sql_full_column_names",
> "sql_full_metadata",
> "sql_parser_debug",
> diff --git a/test/box/session_settings.result b/test/box/session_settings.result
> index d38707ef8..cf94da48c 100644
> --- a/test/box/session_settings.result
> +++ b/test/box/session_settings.result
> @@ -40,7 +40,7 @@ s:delete({'b'})
> | ---
> | - error: _session_settings space does not support delete()
> | ...
> -s:replace({'sql_defer_foreign_keys', true})
> +s:replace({'sql_defer_foreign_key_changes', true})
> | ---
> | - error: _session_settings space does not support replace()
> | ...
> @@ -54,7 +54,7 @@ s:select()
> | ---
> | - - ['error_marshaling_enabled', false]
> | - ['sql_default_engine', 'memtx']
> - | - ['sql_defer_foreign_keys', false]
> + | - ['sql_defer_foreign_key_changes', false]
> | - ['sql_full_column_names', false]
> | - ['sql_full_metadata', false]
> | - ['sql_parser_debug', false]
> @@ -112,7 +112,7 @@ check_sorting(s, t, 'sql_d')
> check_sorting(s, t, 'sql_v')
> | ---
> | ...
> -check_sorting(s, t, 'sql_defer_foreign_keys')
> +check_sorting(s, t, 'sql_defer_foreign_key_changes')
> | ---
> | ...
>
> @@ -121,9 +121,9 @@ t:drop()
> | ...
>
> -- Check get() method of session_settings space.
> -s:get({'sql_defer_foreign_keys'})
> +s:get({'sql_defer_foreign_key_changes'})
> | ---
> - | - ['sql_defer_foreign_keys', false]
> + | - ['sql_defer_foreign_key_changes', false]
> | ...
> s:get({'sql_recursive_triggers'})
> | ---
> @@ -156,13 +156,13 @@ for key, value in s:pairs() do table.insert(t, {key, value}) end
> -- Check update() method of session_settings space.
>
> -- Correct updates.
> -s:update('sql_defer_foreign_keys', {{'=', 'value', true}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', true}})
> | ---
> - | - ['sql_defer_foreign_keys', true]
> + | - ['sql_defer_foreign_key_changes', true]
> | ...
> -s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}})
> +s:update({'sql_defer_foreign_key_changes'}, {{'=', 2, false}})
> | ---
> - | - ['sql_defer_foreign_keys', false]
> + | - ['sql_defer_foreign_key_changes', false]
> | ...
> s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
> | ---
> @@ -177,116 +177,116 @@ s:update('a', {{'=', 2, 1}})
> | ...
>
> -- Inorrect updates.
> -s:update({{'sql_defer_foreign_keys'}}, {{'=', 'value', true}})
> +s:update({{'sql_defer_foreign_key_changes'}}, {{'=', 'value', true}})
> | ---
> | - error: 'Supplied key type of part 0 does not match index part type: expected string'
> | ...
>
> -s:update('sql_defer_foreign_keys', {'=', 'value', true})
> +s:update('sql_defer_foreign_key_changes', {'=', 'value', true})
> | ---
> | - error: Illegal parameters, update operation must be an array {op,..}
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 'value', true}, {'=', 2, true}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', true}, {'=', 2, true}})
> | ---
> - | - ['sql_defer_foreign_keys', true]
> + | - ['sql_defer_foreign_key_changes', true]
> | ...
> -s:update('sql_defer_foreign_keys', {{}})
> +s:update('sql_defer_foreign_key_changes', {{}})
> | ---
> | - error: Illegal parameters, update operation must be an array {op,..}, got empty
> | array
> | ...
> -s:update('sql_defer_foreign_keys', {{'='}})
> +s:update('sql_defer_foreign_key_changes', {{'='}})
> | ---
> | - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
> | 1'
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 'value'}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value'}})
> | ---
> | - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
> | 2'
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', true, 1}})
> | ---
> | - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
> | 4'
> | ...
>
> -s:update('sql_defer_foreign_keys', {{'+', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'+', 'value', 2}})
> | ---
> | - error: 'Argument type in operation ''+'' on field ''value'' does not match field
> | type: expected a number'
> | ...
> -s:update('sql_defer_foreign_keys', {{'-', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'-', 'value', 2}})
> | ---
> | - error: 'Argument type in operation ''-'' on field ''value'' does not match field
> | type: expected a number'
> | ...
> -s:update('sql_defer_foreign_keys', {{'&', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'&', 'value', 2}})
> | ---
> | - error: 'Argument type in operation ''&'' on field ''value'' does not match field
> | type: expected a positive integer'
> | ...
> -s:update('sql_defer_foreign_keys', {{'|', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'|', 'value', 2}})
> | ---
> | - error: 'Argument type in operation ''|'' on field ''value'' does not match field
> | type: expected a positive integer'
> | ...
> -s:update('sql_defer_foreign_keys', {{'^', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'^', 'value', 2}})
> | ---
> | - error: 'Argument type in operation ''^'' on field ''value'' does not match field
> | type: expected a positive integer'
> | ...
> -s:update('sql_defer_foreign_keys', {{'!', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'!', 'value', 2}})
> | ---
> | - error: Tuple field count 3 does not match space field count 2
> | ...
> -s:update('sql_defer_foreign_keys', {{'#', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'#', 'value', 2}})
> | ---
> | - error: Tuple field count 1 does not match space field count 2
> | ...
> -s:update('sql_defer_foreign_keys', {{1, 'value', true}})
> +s:update('sql_defer_foreign_key_changes', {{1, 'value', true}})
> | ---
> | - error: Illegal parameters, update operation name must be a string
> | ...
> -s:update('sql_defer_foreign_keys', {{{1}, 'value', true}})
> +s:update('sql_defer_foreign_key_changes', {{{1}, 'value', true}})
> | ---
> | - error: Illegal parameters, update operation name must be a string
> | ...
>
> -s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}})
> +s:update('sql_defer_foreign_key_changes', {{'=', {'value'}, true}})
> | ---
> | - error: Illegal parameters, field id must be a number or a string
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 1, 'new_key'}})
> | ---
> | - error: Attempt to modify a tuple field which is part of index 'primary' in space
> | '_session_settings'
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'name', 'new_key'}})
> | ---
> | - error: Attempt to modify a tuple field which is part of index 'primary' in space
> | '_session_settings'
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 3, true}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 3, true}})
> | ---
> | - error: Tuple field count 3 does not match space field count 2
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'some text', true}})
> | ---
> | - error: Field 'some text' was not found in the tuple
> | ...
>
> -s:update('sql_defer_foreign_keys', {{'=', 'value', 1}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', 1}})
> | ---
> - | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
> + | - error: Session setting sql_defer_foreign_key_changes expected a value of type boolean
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', {1}}})
> | ---
> - | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
> + | - error: Session setting sql_defer_foreign_key_changes expected a value of type boolean
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', '1'}})
> | ---
> - | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
> + | - error: Session setting sql_defer_foreign_key_changes expected a value of type boolean
> | ...
>
> -- gh-4711: Provide a user-friendly frontend for accessing session settings.
> @@ -313,18 +313,18 @@ s:get('sql_default_engine').value
> | ---
> | - memtx
> | ...
> -settings.sql_defer_foreign_keys = true
> +settings.sql_defer_foreign_key_changes = true
> | ---
> | ...
> -s:get('sql_defer_foreign_keys').value
> +s:get('sql_defer_foreign_key_changes').value
> | ---
> | - true
> | ...
> -s:update('sql_defer_foreign_keys', {{'=', 2, false}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 2, false}})
> | ---
> - | - ['sql_defer_foreign_keys', false]
> + | - ['sql_defer_foreign_key_changes', false]
> | ...
> -settings.sql_defer_foreign_keys
> +settings.sql_defer_foreign_key_changes
> | ---
> | - false
> | ...
> @@ -345,19 +345,19 @@ s:get('sql_default_engine').value
> | ---
> | - memtx
> | ...
> -box.execute([[set session "sql_defer_foreign_keys" = true]])
> +box.execute([[set session "sql_defer_foreign_key_changes" = true]])
> | ---
> | - row_count: 1
> | ...
> -s:get('sql_defer_foreign_keys').value
> +s:get('sql_defer_foreign_key_changes').value
> | ---
> | - true
> | ...
> -box.execute([[set session "sql_defer_foreign_keys" = false]])
> +box.execute([[set session "sql_defer_foreign_key_changes" = false]])
> | ---
> | - row_count: 1
> | ...
> -s:get('sql_defer_foreign_keys').value
> +s:get('sql_defer_foreign_key_changes').value
> | ---
> | - false
> | ...
> @@ -366,9 +366,9 @@ settings.sql_default_engine = true
> | ---
> | - error: Session setting sql_default_engine expected a value of type string
> | ...
> -settings.sql_defer_foreign_keys = 'false'
> +settings.sql_defer_foreign_key_changes = 'false'
> | ---
> - | - error: Session setting sql_defer_foreign_keys expected a value of type boolean
> + | - error: Session setting sql_defer_foreign_key_changes expected a value of type boolean
> | ...
> settings.sql_parser_debug = 'string'
> | ---
> @@ -393,8 +393,8 @@ box.execute([[set session "sql_default_engine" = true]])
> | - null
> | - Session setting sql_default_engine expected a value of type string
> | ...
> -box.execute([[set session "sql_defer_foreign_keys" = 'true']])
> +box.execute([[set session "sql_defer_foreign_key_changes" = 'true']])
> | ---
> | - null
> - | - Session setting sql_defer_foreign_keys expected a value of type boolean
> + | - Session setting sql_defer_foreign_key_changes expected a value of type boolean
> | ...
> diff --git a/test/box/session_settings.test.lua b/test/box/session_settings.test.lua
> index 70c1e00be..468b04d56 100644
> --- a/test/box/session_settings.test.lua
> +++ b/test/box/session_settings.test.lua
> @@ -17,7 +17,7 @@ s:drop()
> s:create_index('a')
> s:insert({'a', 1})
> s:delete({'b'})
> -s:replace({'sql_defer_foreign_keys', true})
> +s:replace({'sql_defer_foreign_key_changes', true})
>
> --
> -- Check select() method of session_settings space. Should work
> @@ -52,12 +52,12 @@ check_sorting(s, t)
> check_sorting(s, t, 'abcde')
> check_sorting(s, t, 'sql_d')
> check_sorting(s, t, 'sql_v')
> -check_sorting(s, t, 'sql_defer_foreign_keys')
> +check_sorting(s, t, 'sql_defer_foreign_key_changes')
>
> t:drop()
>
> -- Check get() method of session_settings space.
> -s:get({'sql_defer_foreign_keys'})
> +s:get({'sql_defer_foreign_key_changes'})
> s:get({'sql_recursive_triggers'})
> s:get({'sql_reverse_unordered_selects'})
> s:get({'sql_default_engine'})
> @@ -71,41 +71,41 @@ for key, value in s:pairs() do table.insert(t, {key, value}) end
> -- Check update() method of session_settings space.
>
> -- Correct updates.
> -s:update('sql_defer_foreign_keys', {{'=', 'value', true}})
> -s:update({'sql_defer_foreign_keys'}, {{'=', 2, false}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', true}})
> +s:update({'sql_defer_foreign_key_changes'}, {{'=', 2, false}})
> s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
> s:update('sql_default_engine', {{':', 'value', 1, 5, 'memtx'}})
> s:update('a', {{'=', 2, 1}})
>
> -- Inorrect updates.
> -s:update({{'sql_defer_foreign_keys'}}, {{'=', 'value', true}})
> -
> -s:update('sql_defer_foreign_keys', {'=', 'value', true})
> -s:update('sql_defer_foreign_keys', {{'=', 'value', true}, {'=', 2, true}})
> -s:update('sql_defer_foreign_keys', {{}})
> -s:update('sql_defer_foreign_keys', {{'='}})
> -s:update('sql_defer_foreign_keys', {{'=', 'value'}})
> -s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}})
> -
> -s:update('sql_defer_foreign_keys', {{'+', 'value', 2}})
> -s:update('sql_defer_foreign_keys', {{'-', 'value', 2}})
> -s:update('sql_defer_foreign_keys', {{'&', 'value', 2}})
> -s:update('sql_defer_foreign_keys', {{'|', 'value', 2}})
> -s:update('sql_defer_foreign_keys', {{'^', 'value', 2}})
> -s:update('sql_defer_foreign_keys', {{'!', 'value', 2}})
> -s:update('sql_defer_foreign_keys', {{'#', 'value', 2}})
> -s:update('sql_defer_foreign_keys', {{1, 'value', true}})
> -s:update('sql_defer_foreign_keys', {{{1}, 'value', true}})
> -
> -s:update('sql_defer_foreign_keys', {{'=', {'value'}, true}})
> -s:update('sql_defer_foreign_keys', {{'=', 1, 'new_key'}})
> -s:update('sql_defer_foreign_keys', {{'=', 'name', 'new_key'}})
> -s:update('sql_defer_foreign_keys', {{'=', 3, true}})
> -s:update('sql_defer_foreign_keys', {{'=', 'some text', true}})
> -
> -s:update('sql_defer_foreign_keys', {{'=', 'value', 1}})
> -s:update('sql_defer_foreign_keys', {{'=', 'value', {1}}})
> -s:update('sql_defer_foreign_keys', {{'=', 'value', '1'}})
> +s:update({{'sql_defer_foreign_key_changes'}}, {{'=', 'value', true}})
> +
> +s:update('sql_defer_foreign_key_changes', {'=', 'value', true})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', true}, {'=', 2, true}})
> +s:update('sql_defer_foreign_key_changes', {{}})
> +s:update('sql_defer_foreign_key_changes', {{'='}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value'}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', true, 1}})
> +
> +s:update('sql_defer_foreign_key_changes', {{'+', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'-', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'&', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'|', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'^', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'!', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{'#', 'value', 2}})
> +s:update('sql_defer_foreign_key_changes', {{1, 'value', true}})
> +s:update('sql_defer_foreign_key_changes', {{{1}, 'value', true}})
> +
> +s:update('sql_defer_foreign_key_changes', {{'=', {'value'}, true}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 1, 'new_key'}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'name', 'new_key'}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 3, true}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'some text', true}})
> +
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', 1}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', {1}}})
> +s:update('sql_defer_foreign_key_changes', {{'=', 'value', '1'}})
>
> -- gh-4711: Provide a user-friendly frontend for accessing session settings.
> settings = box.session.settings
> @@ -115,22 +115,22 @@ s:update('sql_default_engine', {{'=', 2, 'vinyl'}})
> settings.sql_default_engine
> settings.sql_default_engine = 'memtx'
> s:get('sql_default_engine').value
> -settings.sql_defer_foreign_keys = true
> -s:get('sql_defer_foreign_keys').value
> -s:update('sql_defer_foreign_keys', {{'=', 2, false}})
> -settings.sql_defer_foreign_keys
> +settings.sql_defer_foreign_key_changes = true
> +s:get('sql_defer_foreign_key_changes').value
> +s:update('sql_defer_foreign_key_changes', {{'=', 2, false}})
> +settings.sql_defer_foreign_key_changes
>
> box.execute([[set session "sql_default_engine" = 'vinyl']])
> s:get('sql_default_engine').value
> box.execute([[set session "sql_default_engine" = 'memtx']])
> s:get('sql_default_engine').value
> -box.execute([[set session "sql_defer_foreign_keys" = true]])
> -s:get('sql_defer_foreign_keys').value
> -box.execute([[set session "sql_defer_foreign_keys" = false]])
> -s:get('sql_defer_foreign_keys').value
> +box.execute([[set session "sql_defer_foreign_key_changes" = true]])
> +s:get('sql_defer_foreign_key_changes').value
> +box.execute([[set session "sql_defer_foreign_key_changes" = false]])
> +s:get('sql_defer_foreign_key_changes').value
>
> settings.sql_default_engine = true
> -settings.sql_defer_foreign_keys = 'false'
> +settings.sql_defer_foreign_key_changes = 'false'
> settings.sql_parser_debug = 'string'
>
> str = string.rep('a', 20 * 1024)
> @@ -138,4 +138,4 @@ box.session.settings.sql_default_engine = str
>
> box.execute([[set session "sql_def_engine" = true]])
> box.execute([[set session "sql_default_engine" = true]])
> -box.execute([[set session "sql_defer_foreign_keys" = 'true']])
> +box.execute([[set session "sql_defer_foreign_key_changes" = 'true']])
> diff --git a/test/sql/transitive-transactions.result b/test/sql/transitive-transactions.result
> index 91c35a309..c65c123ea 100644
> --- a/test/sql/transitive-transactions.result
> +++ b/test/sql/transitive-transactions.result
> @@ -115,7 +115,7 @@ box.space.PARENT:select();
> ---
> - - [1, 1]
> ...
> -box.space._session_settings:update('sql_defer_foreign_keys', {{'=', 2, true}})
> +box.space._session_settings:update('sql_defer_foreign_key_changes', {{'=', 2, true}})
> box.rollback()
> fk_defer();
> ---
> @@ -130,7 +130,7 @@ box.space.PARENT:select();
> - [2, 2]
> ...
> -- Cleanup
> -box.space._session_settings:update('sql_defer_foreign_keys', {{'=', 2, false}})
> +box.space._session_settings:update('sql_defer_foreign_key_changes', {{'=', 2, false}})
>
> box.execute('DROP TABLE child;');
> ---
> diff --git a/test/sql/transitive-transactions.test.lua b/test/sql/transitive-transactions.test.lua
> index 5565de7fe..e9e7bab92 100644
> --- a/test/sql/transitive-transactions.test.lua
> +++ b/test/sql/transitive-transactions.test.lua
> @@ -62,12 +62,12 @@ end;
> fk_defer();
> box.space.CHILD:select();
> box.space.PARENT:select();
> -box.space._session_settings:update('sql_defer_foreign_keys', {{'=', 2, true}})
> +box.space._session_settings:update('sql_defer_foreign_key_changes', {{'=', 2, true}})
> box.rollback()
> fk_defer();
> box.space.CHILD:select();
> box.space.PARENT:select();
> -box.space._session_settings:update('sql_defer_foreign_keys', {{'=', 2, false}})
> +box.space._session_settings:update('sql_defer_foreign_key_changes', {{'=', 2, false}})
>
> -- Cleanup
> box.execute('DROP TABLE child;');
> --
> 2.21.0 (Apple Git-122)
>
4. Please add tests that show that the old name no longer works
and the new name works.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages
2020-07-14 16:07 ` Mergen Imeev
@ 2020-10-08 16:17 ` Roman Khabibov
2020-11-03 20:31 ` Mergen Imeev
0 siblings, 1 reply; 12+ messages in thread
From: Roman Khabibov @ 2020-10-08 16:17 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches
Hi! Thanks for the review. The second patch was dropped.
> On Jul 14, 2020, at 19:07, Mergen Imeev <imeevma@tarantool.org> wrote:
>
> See 1 comment below.
>
> On Wed, Jul 08, 2020 at 02:47:17PM +0300, Roman Khabibov wrote:
>> Print the true name of _session_settings space in error messages.
>>
>> Follow up #4511
> 1. Please write the actual issue number here.
>
>> ---
>> src/box/session_settings.c | 8 ++++----
>> test/box/session_settings.result | 8 ++++----
>> 2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
>> index dbbbf2461..2f4b8c2f0 100644
>> --- a/src/box/session_settings.c
>> +++ b/src/box/session_settings.c
>> @@ -301,7 +301,7 @@ session_settings_space_execute_replace(struct space *space, struct txn *txn,
>> (void)txn;
>> (void)request;
>> (void)result;
>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>> "replace()");
>> return -1;
>> }
>> @@ -315,7 +315,7 @@ session_settings_space_execute_delete(struct space *space, struct txn *txn,
>> (void)txn;
>> (void)request;
>> (void)result;
>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>> "delete()");
>> return -1;
>> }
>> @@ -388,7 +388,7 @@ session_settings_space_execute_upsert(struct space *space, struct txn *txn,
>> (void)space;
>> (void)txn;
>> (void)request;
>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>> "upsert()");
>> return -1;
>> }
>> @@ -398,7 +398,7 @@ session_settings_space_create_index(struct space *space, struct index_def *def)
>> {
>> assert(space->def->id == BOX_SESSION_SETTINGS_ID);
>> if (def->iid != 0) {
>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>> "create_index()");
>> return NULL;
>> }
>> diff --git a/test/box/session_settings.result b/test/box/session_settings.result
>> index 149cc4bd5..d38707ef8 100644
>> --- a/test/box/session_settings.result
>> +++ b/test/box/session_settings.result
>> @@ -30,19 +30,19 @@ s:drop()
>> --
>> s:create_index('a')
>> | ---
>> - | - error: Session_settings space does not support create_index()
>> + | - error: _session_settings space does not support create_index()
>> | ...
>> s:insert({'a', 1})
>> | ---
>> - | - error: Session_settings space does not support replace()
>> + | - error: _session_settings space does not support replace()
>> | ...
>> s:delete({'b'})
>> | ---
>> - | - error: Session_settings space does not support delete()
>> + | - error: _session_settings space does not support delete()
>> | ...
>> s:replace({'sql_defer_foreign_keys', true})
>> | ---
>> - | - error: Session_settings space does not support replace()
>> + | - error: _session_settings space does not support replace()
>> | ...
>>
>> --
>> --
>> 2.21.0 (Apple Git-122)
>>
commit 340007d7fbc1329f57dc1c3ac9f8ab60b75e6b30
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date: Tue Jul 7 18:15:02 2020 +0300
sql: fix _session_settings name in error messages
Print the true name of _session_settings space in error messages.
Closes #4732
diff --git a/src/box/session_settings.c b/src/box/session_settings.c
index dbbbf2461..2f4b8c2f0 100644
--- a/src/box/session_settings.c
+++ b/src/box/session_settings.c
@@ -301,7 +301,7 @@ session_settings_space_execute_replace(struct space *space, struct txn *txn,
(void)txn;
(void)request;
(void)result;
- diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
"replace()");
return -1;
}
@@ -315,7 +315,7 @@ session_settings_space_execute_delete(struct space *space, struct txn *txn,
(void)txn;
(void)request;
(void)result;
- diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
"delete()");
return -1;
}
@@ -388,7 +388,7 @@ session_settings_space_execute_upsert(struct space *space, struct txn *txn,
(void)space;
(void)txn;
(void)request;
- diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
"upsert()");
return -1;
}
@@ -398,7 +398,7 @@ session_settings_space_create_index(struct space *space, struct index_def *def)
{
assert(space->def->id == BOX_SESSION_SETTINGS_ID);
if (def->iid != 0) {
- diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
+ diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
"create_index()");
return NULL;
}
diff --git a/test/box/session_settings.result b/test/box/session_settings.result
index 149cc4bd5..d38707ef8 100644
--- a/test/box/session_settings.result
+++ b/test/box/session_settings.result
@@ -30,19 +30,19 @@ s:drop()
--
s:create_index('a')
| ---
- | - error: Session_settings space does not support create_index()
+ | - error: _session_settings space does not support create_index()
| ...
s:insert({'a', 1})
| ---
- | - error: Session_settings space does not support replace()
+ | - error: _session_settings space does not support replace()
| ...
s:delete({'b'})
| ---
- | - error: Session_settings space does not support delete()
+ | - error: _session_settings space does not support delete()
| ...
s:replace({'sql_defer_foreign_keys', true})
| ---
- | - error: Session_settings space does not support replace()
+ | - error: _session_settings space does not support replace()
| ...
--
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages
2020-10-08 16:17 ` Roman Khabibov
@ 2020-11-03 20:31 ` Mergen Imeev
2020-11-10 16:12 ` Roman Khabibov
0 siblings, 1 reply; 12+ messages in thread
From: Mergen Imeev @ 2020-11-03 20:31 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
Hi! Thank you for the patch. I see no test here, but since it is not a bug, this
should be fine. LGTM
On Thu, Oct 08, 2020 at 07:17:11PM +0300, Roman Khabibov wrote:
> Hi! Thanks for the review. The second patch was dropped.
>
> > On Jul 14, 2020, at 19:07, Mergen Imeev <imeevma@tarantool.org> wrote:
> >
> > See 1 comment below.
> >
> > On Wed, Jul 08, 2020 at 02:47:17PM +0300, Roman Khabibov wrote:
> >> Print the true name of _session_settings space in error messages.
> >>
> >> Follow up #4511
> > 1. Please write the actual issue number here.
> >
> >> ---
> >> src/box/session_settings.c | 8 ++++----
> >> test/box/session_settings.result | 8 ++++----
> >> 2 files changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
> >> index dbbbf2461..2f4b8c2f0 100644
> >> --- a/src/box/session_settings.c
> >> +++ b/src/box/session_settings.c
> >> @@ -301,7 +301,7 @@ session_settings_space_execute_replace(struct space *space, struct txn *txn,
> >> (void)txn;
> >> (void)request;
> >> (void)result;
> >> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> >> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> >> "replace()");
> >> return -1;
> >> }
> >> @@ -315,7 +315,7 @@ session_settings_space_execute_delete(struct space *space, struct txn *txn,
> >> (void)txn;
> >> (void)request;
> >> (void)result;
> >> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> >> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> >> "delete()");
> >> return -1;
> >> }
> >> @@ -388,7 +388,7 @@ session_settings_space_execute_upsert(struct space *space, struct txn *txn,
> >> (void)space;
> >> (void)txn;
> >> (void)request;
> >> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> >> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> >> "upsert()");
> >> return -1;
> >> }
> >> @@ -398,7 +398,7 @@ session_settings_space_create_index(struct space *space, struct index_def *def)
> >> {
> >> assert(space->def->id == BOX_SESSION_SETTINGS_ID);
> >> if (def->iid != 0) {
> >> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> >> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> >> "create_index()");
> >> return NULL;
> >> }
> >> diff --git a/test/box/session_settings.result b/test/box/session_settings.result
> >> index 149cc4bd5..d38707ef8 100644
> >> --- a/test/box/session_settings.result
> >> +++ b/test/box/session_settings.result
> >> @@ -30,19 +30,19 @@ s:drop()
> >> --
> >> s:create_index('a')
> >> | ---
> >> - | - error: Session_settings space does not support create_index()
> >> + | - error: _session_settings space does not support create_index()
> >> | ...
> >> s:insert({'a', 1})
> >> | ---
> >> - | - error: Session_settings space does not support replace()
> >> + | - error: _session_settings space does not support replace()
> >> | ...
> >> s:delete({'b'})
> >> | ---
> >> - | - error: Session_settings space does not support delete()
> >> + | - error: _session_settings space does not support delete()
> >> | ...
> >> s:replace({'sql_defer_foreign_keys', true})
> >> | ---
> >> - | - error: Session_settings space does not support replace()
> >> + | - error: _session_settings space does not support replace()
> >> | ...
> >>
> >> --
> >> --
> >> 2.21.0 (Apple Git-122)
> >>
>
> commit 340007d7fbc1329f57dc1c3ac9f8ab60b75e6b30
> Author: Roman Khabibov <roman.habibov@tarantool.org>
> Date: Tue Jul 7 18:15:02 2020 +0300
>
> sql: fix _session_settings name in error messages
>
> Print the true name of _session_settings space in error messages.
>
> Closes #4732
>
> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
> index dbbbf2461..2f4b8c2f0 100644
> --- a/src/box/session_settings.c
> +++ b/src/box/session_settings.c
> @@ -301,7 +301,7 @@ session_settings_space_execute_replace(struct space *space, struct txn *txn,
> (void)txn;
> (void)request;
> (void)result;
> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> "replace()");
> return -1;
> }
> @@ -315,7 +315,7 @@ session_settings_space_execute_delete(struct space *space, struct txn *txn,
> (void)txn;
> (void)request;
> (void)result;
> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> "delete()");
> return -1;
> }
> @@ -388,7 +388,7 @@ session_settings_space_execute_upsert(struct space *space, struct txn *txn,
> (void)space;
> (void)txn;
> (void)request;
> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> "upsert()");
> return -1;
> }
> @@ -398,7 +398,7 @@ session_settings_space_create_index(struct space *space, struct index_def *def)
> {
> assert(space->def->id == BOX_SESSION_SETTINGS_ID);
> if (def->iid != 0) {
> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
> "create_index()");
> return NULL;
> }
> diff --git a/test/box/session_settings.result b/test/box/session_settings.result
> index 149cc4bd5..d38707ef8 100644
> --- a/test/box/session_settings.result
> +++ b/test/box/session_settings.result
> @@ -30,19 +30,19 @@ s:drop()
> --
> s:create_index('a')
> | ---
> - | - error: Session_settings space does not support create_index()
> + | - error: _session_settings space does not support create_index()
> | ...
> s:insert({'a', 1})
> | ---
> - | - error: Session_settings space does not support replace()
> + | - error: _session_settings space does not support replace()
> | ...
> s:delete({'b'})
> | ---
> - | - error: Session_settings space does not support delete()
> + | - error: _session_settings space does not support delete()
> | ...
> s:replace({'sql_defer_foreign_keys', true})
> | ---
> - | - error: Session_settings space does not support replace()
> + | - error: _session_settings space does not support replace()
> | ...
>
> --
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages
2020-11-03 20:31 ` Mergen Imeev
@ 2020-11-10 16:12 ` Roman Khabibov
0 siblings, 0 replies; 12+ messages in thread
From: Roman Khabibov @ 2020-11-10 16:12 UTC (permalink / raw)
To: Kirill Yukhin; +Cc: tarantool-patches
Thanks. Kirill, can you push?
@ChangeLog:
* Rename Session_settings to _session_settings in error messages (gh-4732).
> On Nov 3, 2020, at 23:31, Mergen Imeev <imeevma@tarantool.org> wrote:
>
> Hi! Thank you for the patch. I see no test here, but since it is not a bug, this
> should be fine. LGTM
>
> On Thu, Oct 08, 2020 at 07:17:11PM +0300, Roman Khabibov wrote:
>> Hi! Thanks for the review. The second patch was dropped.
>>
>>> On Jul 14, 2020, at 19:07, Mergen Imeev <imeevma@tarantool.org> wrote:
>>>
>>> See 1 comment below.
>>>
>>> On Wed, Jul 08, 2020 at 02:47:17PM +0300, Roman Khabibov wrote:
>>>> Print the true name of _session_settings space in error messages.
>>>>
>>>> Follow up #4511
>>> 1. Please write the actual issue number here.
>>>
>>>> ---
>>>> src/box/session_settings.c | 8 ++++----
>>>> test/box/session_settings.result | 8 ++++----
>>>> 2 files changed, 8 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
>>>> index dbbbf2461..2f4b8c2f0 100644
>>>> --- a/src/box/session_settings.c
>>>> +++ b/src/box/session_settings.c
>>>> @@ -301,7 +301,7 @@ session_settings_space_execute_replace(struct space *space, struct txn *txn,
>>>> (void)txn;
>>>> (void)request;
>>>> (void)result;
>>>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>>>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>>>> "replace()");
>>>> return -1;
>>>> }
>>>> @@ -315,7 +315,7 @@ session_settings_space_execute_delete(struct space *space, struct txn *txn,
>>>> (void)txn;
>>>> (void)request;
>>>> (void)result;
>>>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>>>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>>>> "delete()");
>>>> return -1;
>>>> }
>>>> @@ -388,7 +388,7 @@ session_settings_space_execute_upsert(struct space *space, struct txn *txn,
>>>> (void)space;
>>>> (void)txn;
>>>> (void)request;
>>>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>>>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>>>> "upsert()");
>>>> return -1;
>>>> }
>>>> @@ -398,7 +398,7 @@ session_settings_space_create_index(struct space *space, struct index_def *def)
>>>> {
>>>> assert(space->def->id == BOX_SESSION_SETTINGS_ID);
>>>> if (def->iid != 0) {
>>>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>>>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>>>> "create_index()");
>>>> return NULL;
>>>> }
>>>> diff --git a/test/box/session_settings.result b/test/box/session_settings.result
>>>> index 149cc4bd5..d38707ef8 100644
>>>> --- a/test/box/session_settings.result
>>>> +++ b/test/box/session_settings.result
>>>> @@ -30,19 +30,19 @@ s:drop()
>>>> --
>>>> s:create_index('a')
>>>> | ---
>>>> - | - error: Session_settings space does not support create_index()
>>>> + | - error: _session_settings space does not support create_index()
>>>> | ...
>>>> s:insert({'a', 1})
>>>> | ---
>>>> - | - error: Session_settings space does not support replace()
>>>> + | - error: _session_settings space does not support replace()
>>>> | ...
>>>> s:delete({'b'})
>>>> | ---
>>>> - | - error: Session_settings space does not support delete()
>>>> + | - error: _session_settings space does not support delete()
>>>> | ...
>>>> s:replace({'sql_defer_foreign_keys', true})
>>>> | ---
>>>> - | - error: Session_settings space does not support replace()
>>>> + | - error: _session_settings space does not support replace()
>>>> | ...
>>>>
>>>> --
>>>> --
>>>> 2.21.0 (Apple Git-122)
>>>>
>>
>> commit 340007d7fbc1329f57dc1c3ac9f8ab60b75e6b30
>> Author: Roman Khabibov <roman.habibov@tarantool.org>
>> Date: Tue Jul 7 18:15:02 2020 +0300
>>
>> sql: fix _session_settings name in error messages
>>
>> Print the true name of _session_settings space in error messages.
>>
>> Closes #4732
>>
>> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
>> index dbbbf2461..2f4b8c2f0 100644
>> --- a/src/box/session_settings.c
>> +++ b/src/box/session_settings.c
>> @@ -301,7 +301,7 @@ session_settings_space_execute_replace(struct space *space, struct txn *txn,
>> (void)txn;
>> (void)request;
>> (void)result;
>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>> "replace()");
>> return -1;
>> }
>> @@ -315,7 +315,7 @@ session_settings_space_execute_delete(struct space *space, struct txn *txn,
>> (void)txn;
>> (void)request;
>> (void)result;
>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>> "delete()");
>> return -1;
>> }
>> @@ -388,7 +388,7 @@ session_settings_space_execute_upsert(struct space *space, struct txn *txn,
>> (void)space;
>> (void)txn;
>> (void)request;
>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>> "upsert()");
>> return -1;
>> }
>> @@ -398,7 +398,7 @@ session_settings_space_create_index(struct space *space, struct index_def *def)
>> {
>> assert(space->def->id == BOX_SESSION_SETTINGS_ID);
>> if (def->iid != 0) {
>> - diag_set(ClientError, ER_UNSUPPORTED, "Session_settings space",
>> + diag_set(ClientError, ER_UNSUPPORTED, "_session_settings space",
>> "create_index()");
>> return NULL;
>> }
>> diff --git a/test/box/session_settings.result b/test/box/session_settings.result
>> index 149cc4bd5..d38707ef8 100644
>> --- a/test/box/session_settings.result
>> +++ b/test/box/session_settings.result
>> @@ -30,19 +30,19 @@ s:drop()
>> --
>> s:create_index('a')
>> | ---
>> - | - error: Session_settings space does not support create_index()
>> + | - error: _session_settings space does not support create_index()
>> | ...
>> s:insert({'a', 1})
>> | ---
>> - | - error: Session_settings space does not support replace()
>> + | - error: _session_settings space does not support replace()
>> | ...
>> s:delete({'b'})
>> | ---
>> - | - error: Session_settings space does not support delete()
>> + | - error: _session_settings space does not support delete()
>> | ...
>> s:replace({'sql_defer_foreign_keys', true})
>> | ---
>> - | - error: Session_settings space does not support replace()
>> + | - error: _session_settings space does not support replace()
>> | ...
>>
>> --
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space
2020-07-08 11:47 [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space Roman Khabibov
` (2 preceding siblings ...)
2020-07-14 16:01 ` [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space Mergen Imeev
@ 2020-11-26 9:51 ` Kirill Yukhin
3 siblings, 0 replies; 12+ messages in thread
From: Kirill Yukhin @ 2020-11-26 9:51 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
Hello,
On 08 Jul 14:47, Roman Khabibov wrote:
> @ChangeLog
> - Fix _session_settings name in error messages
> - Raname "sql_defer_foreign_keys" setting to
> "sql_defer_foreign_key_changes".
>
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4732-ss-space
> Issue: https://github.com/tarantool/tarantool/issues/4732
I've checked your patch into 2.5, 2.6 and master.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-11-26 9:51 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-08 11:47 [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space Roman Khabibov
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 1/2] sql: fix _session_settings name in error messages Roman Khabibov
2020-07-14 16:07 ` Mergen Imeev
2020-10-08 16:17 ` Roman Khabibov
2020-11-03 20:31 ` Mergen Imeev
2020-11-10 16:12 ` Roman Khabibov
2020-07-08 11:47 ` [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name Roman Khabibov
2020-07-12 15:31 ` Vladislav Shpilevoy
2020-07-12 22:11 ` Nikita Pettik
2020-07-14 16:22 ` Mergen Imeev
2020-07-14 16:01 ` [Tarantool-patches] [PATCH 0/2] Follow-up for "_session_settings" space Mergen Imeev
2020-11-26 9:51 ` Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox