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

UDTs of type map[string]interface{} errors out on marshalUDT when missing fields #841

Closed
aravindvs opened this issue Nov 28, 2016 · 0 comments

Comments

@aravindvs
Copy link

aravindvs commented Nov 28, 2016

This is very similar to #757. With that issue resolved, unmarshalUDT never panics. But CQL spec states this (as mentioned on the other commit as well):
"
7. User Defined Types
...
A UDT value will generally have one value
for each field of the type it represents, but it is allowed to have less values than
the type has fields.

Within a user-defined type value, all data types should use the v3 protocol
serialization format.
"
Now according to the aforementioned spec, if a UDT is of type map[string]interface{}, each "key" in the map is a field
and shouldn't we be behaving the same way like we are missing a field?

Right now gocql throws an error. I added a simple test case to illustrate this. I ran the test as follows and it should fail on top of master:
$go test -tags=integration -proto=4 -run "UDT" -v .

...

=== RUN   TestUDT_UpdateFieldMissingInterface

--- FAIL: TestUDT_UpdateFieldMissingInterface (0.16s)

udt_test.go:451: missing UDT field in map: data_v2

...

Is this behavior intentional? If so, why? Shouldn't this be treated like any other missing field in a UDT?

func TestUDT_UpdateFieldMissingInterface(t *testing.T) {
        if *flagProto < protoVersion3 {
                t.Skip("UDT are only available on protocol >= 3")
        }

        // This test tries to simulate an update to an existing UDT
        // of type map[string]interface{}
        // When we try to update an altered type with a missing key
        // in the map, we get an error, when we probably should treat it with a
        // nil value.

        updateFieldUDTMap := make(map[string]interface{})
        updateFieldUDTMap["data_v1"] = "old_version"

        session := createSession(t)
        defer session.Close()

        // 1. Create a UDT with just 2 fields
        err := createTable(session, `CREATE TYPE gocql_test.update_field_udt_map(
                data_v1 text);`)
        if err != nil {
                t.Fatal(err)
        }

        // 2.  Create a table with the above UDT as a map
        err = createTable(session, `CREATE TABLE gocql_test.update_field_map(
        id uuid,
        udt_col frozen<update_field_udt_map>,
        primary key(id));`)
        if err != nil {
                t.Fatal(err)
        }

        // 3. ALTER UDT to add a new field
        if err := createTable(session, `ALTER TYPE gocql_test.update_field_udt_map ADD data_v2 text;`); err != nil {
                t.Fatal(err)
        }
        
        // 4. Now try to Insert into the table, a UDT without the new key we "altered" above
        id := TimeUUID()
        err = session.Query("INSERT INTO update_field_map(id, udt_col) VALUES(?, ?)", id, updateFieldUDTMap).Exec()
        if err != nil {
                t.Fatal(err)
        }
}
Zariel added a commit that referenced this issue Nov 29, 2016
When inserting a UDT via a map, allow the map to be a subset of the UDT.

Fixes #841
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