[Tarantool-patches] [PATCH 2/2] sql: clarify "sql_defer_foreign_keys" setting name

Tue Jul 14 19:22:16 MSK 2020

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.