From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4002A445320 for ; Tue, 14 Jul 2020 19:22:18 +0300 (MSK) Date: Tue, 14 Jul 2020 19:22:16 +0300 From: Mergen Imeev Message-ID: <20200714162216.GC250285@tarantool.org> References: <20200708114718.38910-1-roman.habibov@tarantool.org> <20200708114718.38910-3-roman.habibov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200708114718.38910-3-roman.habibov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org 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.