config/schema: support field deletion using :set()

This commit implements the `<schema object>:set()` algorithm in a more
accurate way and it solves several drawbacks of the previous
implementation.

* It was impossible to set a field that is nested to a record or a map
  that has the box.NULL value (tarantool#10190).
* It was impossible to set a field to the box.NULL value (tarantool#10193).
* It was impossible to delete a field, now `nil` RHS value means the
  deletion (tarantool#10194).

Fixes tarantool#10190
Fixes tarantool#10193
Fixes tarantool#10194

NO_DOC=Included into tarantool/doc#4279

Author: Totktonada

changelogs/unreleased/config-schema-set-to-null-or-nil.md

@@ -0,0 +1,4 @@
+## feature/config
+
+* Allow to delete a field or set it to `box.NULL` using the `<schema
+  object>:set()` method (gh-10190, gh-10193, gh-10194).

src/box/lua/config/utils/schema.lua

@@ -763,9 +763,17 @@ end
 local function set_impl(schema, data, rhs, ctx)
     -- The journey is finished. Validate and return the new value.
     if #ctx.journey == 0 then
+        -- Validate `rhs` against the schema node.
+        --
+        -- Allow nil/box.NULL. We reach this code only when the
+        -- given `rhs` value is a field of a record or a map. The
+        -- fields are allowed to be nil/box.NULL.
+        --
         -- Call validate_impl() directly to don't construct a
         -- schema object.
-        validate_impl(schema, rhs, ctx)
+        if rhs ~= nil then
+            validate_impl(schema, rhs, ctx)
+        end
         return rhs
     end
 
@@ -775,26 +783,52 @@ local function set_impl(schema, data, rhs, ctx)
     if is_scalar(schema) then
         walkthrough_error(ctx, 'Attempt to index a scalar value of type %s ' ..
             'by field %q', schema.type, requested_field)
-    elseif schema.type == 'record' then
+    elseif schema.type == 'record' or schema.type == 'map' then
+        -- Traverse down a record/a map that has nil/box.NULL
+        -- value to create intermediate tables on the path to
+        -- `rhs` and to validate the path against the schema.
         walkthrough_enter(ctx, requested_field)
-        local field_def = schema.fields[requested_field]
-        if field_def == nil then
-            walkthrough_error(ctx, 'No such field in the schema')
+
+        -- A schema of the requested field.
+        local field_def
+        if schema.type == 'record' then
+            field_def = schema.fields[requested_field]
+            if field_def == nil then
+                walkthrough_error(ctx, 'No such field in the schema')
+            end
+        else
+            assert(schema.type == 'map')
+            field_def = schema.value
         end
+        assert(field_def ~= nil)
 
-        walkthrough_assert_table(ctx, schema, data)
-        local field_value = data[requested_field] or {}
-        table.remove(ctx.journey, 1)
-        data[requested_field] = set_impl(field_def, field_value, rhs, ctx)
-        return data
-    elseif schema.type == 'map' then
-        walkthrough_enter(ctx, requested_field)
-        local field_def = schema.value
+        -- Current value of the requested field.
+        local field_value
+        if data ~= nil then
+            walkthrough_assert_table(ctx, schema, data)
+            field_value = data[requested_field]
+        end
 
-        walkthrough_assert_table(ctx, schema, data)
-        local field_value = data[requested_field] or {}
+        -- The value of the field may be changed to `rhs` or
+        -- from nil/box.NULL to a newly created table.
         table.remove(ctx.journey, 1)
-        data[requested_field] = set_impl(field_def, field_value, rhs, ctx)
+        local new_field_value = set_impl(field_def, field_value, rhs, ctx)
+
+        if type(new_field_value) == 'nil' then
+            -- Delete the field. Keeps nil and box.NULL, modifies
+            -- a table.
+            if data ~= nil then -- table
+                data[requested_field] = nil
+            end
+        else
+            -- Assign the field. Replaces nil and box.NULL with
+            -- a newly created table, modifies a table.
+            if data == nil then -- nil or box.NULL
+                data = {}
+            end
+            data[requested_field] = new_field_value
+        end
+
         return data
     elseif schema.type == 'array' then
         -- TODO: Support 'foo[1]' and `{'foo', 1}` paths. See the
@@ -833,6 +867,40 @@ end
 -- * A scalar of the 'any' type can't be indexed, even when it is
 --   a table. It is OK to set the whole value of the 'any'
 --   type.
+-- * Assignment of a non-nil `rhs` value creates intermediate
+--   tables over the given `path` instead of nil or box.NULL
+--   values.
+--
+-- Field deletion
+-- --------------
+--
+-- If `rhs` is nil, it means deletion of the pointed field.
+--
+-- How it works (in examples):
+--
+-- Intermediate tables are not created:
+--
+--  | local myschema = <...>
+--  | local data = {}
+--  | myschema:set(data, 'foo.bar', nil)
+--  | -- data is {}, not {foo = {}}
+--
+-- Existing tables on the `path` are not removed:
+--
+--  | local myschema = <...>
+--  | local data = {
+--  |     instances = {
+--  |         foo = {x = 1},
+--  |         bar = {},
+--  |     },
+--  | }
+--  | myschema:set(data, 'instances.foo.x', nil)
+--  | -- data is {
+--  | --     instances = {
+--  | --         foo = {},
+--  | --         bar = {},
+--  | --     },
+--  | -- }
 function methods.set(self, data, path, rhs)
     local schema = rawget(self, 'schema')
 

test/config-luatest/cbuilder.lua

@@ -125,24 +125,6 @@ local function cbuilder_set_replicaset_option(self, path, value)
         'replicasets', self._replicaset,
     }, path:split('.')):totable()
 
-    -- <schema object>:set() validation is too tight. Workaround
-    -- it. Maybe we should reconsider this :set() behavior in a
-    -- future.
-    if value == nil then
-        local cur = self._config
-        for i = 1, #path - 1 do
-            -- Create missed fields.
-            local component = path[i]
-            if cur[component] == nil then
-                cur[component] = {}
-            end
-
-            cur = cur[component]
-        end
-        cur[path[#path]] = value
-        return self
-    end
-
     cluster_config:set(self._config, path, value)
     return self
 end

test/config-luatest/schema_test.lua

@@ -1280,6 +1280,74 @@ g.test_set_record_field = function()
     t.assert_equals(data, {foo = {bar = 'mydata'}})
 end
 
+-- gh-10193: verify that record's field can be set to box.NULL.
+g.test_set_record_field_to_null = function()
+    local s = schema.new('myschema', schema.record({
+        foo = schema.record({
+            bar = schema.scalar({type = 'string'}),
+        }),
+    }))
+
+    local data = {}
+    s:set(data, 'foo', box.NULL)
+    t.assert_type(data.foo, 'cdata')
+
+    local data = {}
+    s:set(data, 'foo.bar', box.NULL)
+    t.assert_type(data.foo, 'table')
+    t.assert_type(data.foo.bar, 'cdata')
+end
+
+-- gh-10190: verify that box.NULL record is replaced by a table if
+-- a nested field is set.
+g.test_set_over_null_record = function()
+    local s = schema.new('myschema', schema.record({
+        foo = schema.record({
+            bar = schema.scalar({type = 'string'}),
+        }),
+    }))
+
+    local data = {foo = box.NULL}
+    s:set(data, 'foo.bar', 'mydata')
+    t.assert_equals(data, {foo = {bar = 'mydata'}})
+end
+
+-- gh-10194: verify that a record's field can be deleted.
+g.test_set_record_field_to_nil = function()
+    local s = schema.new('myschema', schema.record({
+        foo = schema.record({
+            bar = schema.scalar({type = 'string'}),
+            baz = schema.scalar({type = 'string'}),
+        }),
+    }))
+
+    -- Basic scenario (no-op).
+    local data = {}
+    s:set(data, 'foo', nil)
+    t.assert_equals(data, {})
+
+    -- Basic scenario (actual deletion).
+    local data = {foo = {bar = 'x', baz = 'y'}}
+    s:set(data, 'foo.bar', nil)
+    t.assert_equals(data, {foo = {baz = 'y'}})
+
+    -- Don't create intermediate tables.
+    local data = {}
+    s:set(data, 'foo.bar', nil)
+    t.assert_equals(data, {})
+
+    -- Don't drop existing tables.
+    local data = {foo = {bar = 'x'}}
+    s:set(data, 'foo.bar', nil)
+    t.assert_equals(data, {foo = {}})
+
+    -- box.NULL is kept if a deletion of a nested field is
+    -- requested.
+    local data = {foo = box.NULL}
+    s:set(data, 'foo.bar', nil)
+    t.assert_type(data.foo, 'cdata')
+end
+
 -- Verify that there are no problems with composite data in a
 -- scalar of the any type.
 g.test_set_record_field_any = function()
@@ -1346,6 +1414,79 @@ g.test_set_map_field = function()
     t.assert_equals(data, {foo = {bar = 'mydata'}})
 end
 
+-- gh-10193: verify that map's field can be set to box.NULL.
+g.test_set_map_field_to_null = function()
+    local s = schema.new('myschema', schema.map({
+        key = schema.scalar({type = 'string'}),
+        value = schema.map({
+            key = schema.scalar({type = 'string'}),
+            value = schema.scalar({type = 'string'}),
+        }),
+    }))
+
+    local data = {}
+    s:set(data, 'foo', box.NULL)
+    t.assert_type(data.foo, 'cdata')
+
+    local data = {}
+    s:set(data, 'foo.bar', box.NULL)
+    t.assert_type(data.foo, 'table')
+    t.assert_type(data.foo.bar, 'cdata')
+end
+
+-- gh-10190: verify that box.NULL map is replaced by a table if
+-- a nested field is set.
+g.test_set_over_null_map = function()
+    local s = schema.new('myschema', schema.map({
+        key = schema.scalar({type = 'string'}),
+        value = schema.map({
+            key = schema.scalar({type = 'string'}),
+            value = schema.scalar({type = 'string'}),
+        }),
+    }))
+
+    local data = {foo = box.NULL}
+    s:set(data, 'foo.bar', 'mydata')
+    t.assert_equals(data, {foo = {bar = 'mydata'}})
+end
+
+-- gh-10194: verify that a map's field can be deleted.
+g.test_set_map_field_to_nil = function()
+    local s = schema.new('myschema', schema.map({
+        key = schema.scalar({type = 'string'}),
+        value = schema.map({
+            key = schema.scalar({type = 'string'}),
+            value = schema.scalar({type = 'string'}),
+        }),
+    }))
+
+    -- Basic scenario (no-op).
+    local data = {}
+    s:set(data, 'foo', nil)
+    t.assert_equals(data, {})
+
+    -- Basic scenario (actual deletion).
+    local data = {foo = {bar = 'x', baz = 'y'}}
+    s:set(data, 'foo.bar', nil)
+    t.assert_equals(data, {foo = {baz = 'y'}})
+
+    -- Don't create intermediate tables.
+    local data = {}
+    s:set(data, 'foo.bar', nil)
+    t.assert_equals(data, {})
+
+    -- Don't drop existing tables.
+    local data = {foo = {bar = 'x'}}
+    s:set(data, 'foo.bar', nil)
+    t.assert_equals(data, {foo = {}})
+
+    -- box.NULL is kept if a deletion of a nested field is
+    -- requested.
+    local data = {foo = box.NULL}
+    s:set(data, 'foo.bar', nil)
+    t.assert_type(data.foo, 'cdata')
+end
+
 -- Trigger improper API usage error.
 g.test_set_usage = function()
     local s = schema.new('myschema', schema.record({