Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xflatten fails to update two fields in a nested record #19

Closed
Totktonada opened this issue Apr 20, 2017 · 2 comments
Closed

xflatten fails to update two fields in a nested record #19

Totktonada opened this issue Apr 20, 2017 · 2 comments

Comments

@Totktonada
Copy link
Member

How to reproduce:

#!/usr/bin/tarantool

local avro = require('avro_schema')
local log = require('log')
local json = require('json')

local schema={
    name='foo',
    type='record',
    fields={
        {name='bar', type={
            name='bar_t',
            type='record',
            fields={
                {name='value_1', type='string'},
                {name='value_2', type='string'},
            }}  
        },  
    }   
}

local _, schema_handler = avro.create(schema)
local _, model = avro.compile(schema_handler)

-- uncomment to produce case 2
-- model.unflatten({"ccc", "ddd"})

local obj_partial = {bar={value_1="aaa",value_2="bbb"}}
local ok, exps = model.xflatten(obj_partial)
log.info('ok: ' .. json.encode(ok))
log.info('exps: ' .. json.encode(exps))

Example of output:

./avro_case_3.lua: ok: false
./avro_case_3.lua: exps: "Internal error: unknown code"

Or with uncommented unflatten:

./avro_case_3.lua: ok: true
./avro_case_3.lua: exps: [["=",2,"bbb"],"value_2",["=",1,"aaa"]]

gdb output after break in unparse_msgpack() ('unknown code' case, w/ commented unflatten):

(gdb) p nitems
$1 = 10
(gdb) p state->ot
$4 = (uint8_t *) 0x9427d0 "\v\v\022\004\b"
(gdb) p state->ot+5
$9 = (uint8_t *) 0x9427d5 ""
(gdb) p state->ot+6
$10 = (uint8_t *) 0x9427d6 "\v\022\004\b"
(gdb) p *typeid
$14 = 0 '\000'

I will follow up with temporary workarounds in the comments.

@Totktonada
Copy link
Member Author

Totktonada commented Apr 20, 2017

Fast-written & dirty workaround for both cases described above.

local _slow_xflatten_helper
_slow_xflatten_helper = function(schema_names, top_path, obj_partial)
    local exps = {}
    for k, v in pairs(obj_partial) do
        local cur_path = (top_path == '') and k or (top_path .. '.' .. k)
        if type(v) == 'table' then
            local ok, new_exps = _slow_xflatten_helper(
                schema_names, cur_path, v)
            if not ok then
                return false, new_exps
            end
            for _, exp in ipairs(new_exps) do
                exps[#exps + 1] = exp
            end
        else
            local column_no = -1
            for i, name in ipairs(schema_names) do
                if name == cur_path then
                    column_no = i
                    break
                end
            end
            if column_no == -1 then
                local reason = 'cannot find fields name: ' .. cur_path
                return false, reason
            end
            exps[#exps + 1] = {'=', column_no, v}
        end
    end
    return true, exps
end

-- Note: no array, no enum support
local slow_xflatten = function(schema_handle, obj_partial)
    local schema_names = avro.get_names(schema_handle)
    return _slow_xflatten_helper(schema_names, '', obj_partial)
end

local xflatten_wrapper = function(model, schema_handle, obj_partial)
    local valid_exps = function(exps)
        for _, exp in ipairs(exps) do
            local valid_exp = type(exp) == 'table' and #exp == 3
            if valid_exp then
                -- XXX: more checks?
                -- local op, field_no, value = unpack(exp)
            else
                return false
            end
        end
        return true
    end
    
    local ok, exps = model.xflatten(obj_partial)
    local unkn_msg = 'Internal error: unknown code'
    local is_case_1 = not ok and type(exps) == 'string' and exps == unkn_msg
    local is_case_2 = ok and type(exps) == 'table' and not valid_exps(exps)
    if not (is_case_1 or is_case_2) then
        return ok, exps
    end
    return slow_xflatten(schema_handle, obj_partial)
end

Then invoke xflatten_wrapper(model, schema_handle, obj_partial) instead of model.xflatten(obj_partial), where schema_handle is what avro.create() returns and model is what avro.compile() returns (assuming local avro = require('avro_schema')).

@Totktonada
Copy link
Member Author

Works correctly in both cases with debug (w/o optimization):

local _, model = avro.compile({schema_handler, debug=true})

The input of unparse_msgpack w/o optimization:

unparse_msgpack; *typeid: 0x0B (11) -- PUTARRAYC; value: 2   !!!
unparse_msgpack; *typeid: 0x0B (11) -- PUTARRAYC; value: 3
unparse_msgpack; *typeid: 0x12 (18) -- PUTSTRC; value: 1
unparse_msgpack; *typeid: 0x04 (4) -- PUTINT / PUTLONG; value: 2
unparse_msgpack; *typeid: 0x08 (8) -- PUTSTR; value: 3
unparse_msgpack; *typeid: 0x0B (11) -- PUTARRAYC; value: 3
unparse_msgpack; *typeid: 0x12 (18) -- PUTSTRC; value: 1
unparse_msgpack; *typeid: 0x04 (4) -- PUTINT / PUTLONG; value: 1
unparse_msgpack; *typeid: 0x08 (8) -- PUTSTR; value: 3

The input w/ optimization (broken):

unparse_msgpack; *typeid: 0x0B (11) -- PUTARRAYC; value: 3   !!!
unparse_msgpack; *typeid: 0x0B (11) -- PUTARRAYC; value: 3
unparse_msgpack; *typeid: 0x12 (18) -- PUTSTRC; value: 1
unparse_msgpack; *typeid: 0x04 (4) -- PUTINT / PUTLONG; value: 2
unparse_msgpack; *typeid: 0x08 (8) -- PUTSTR; value: 3
unparse_msgpack; *typeid: 0x00 (0) -- (zero); value: 0       !!!
unparse_msgpack; *typeid: 0x0B (11) -- PUTARRAYC; value: 3
unparse_msgpack; *typeid: 0x12 (18) -- PUTSTRC; value: 1
unparse_msgpack; *typeid: 0x04 (4) -- PUTINT / PUTLONG; value: 1
unparse_msgpack; *typeid: 0x08 (8) -- PUTSTR; value: 3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant