Updating map fields is non additive and overwrites previous values about gocassa

[mattheath]

I've also tried this with the MapSetFields modifier - this also replaces the map, whereas MapSetField which upserts a single key:value into the map.

MapSetFields replaces the map with the contents of the provided map:

UPDATE basket_map_id SET Items = ?  WHERE id = ? [map[88d57b18:Creme Egg] 2dcb132a]

MapSetField upserts a single field:

UPDATE basket_map_id SET Items['88d57b18'] = 'Creme Egg'  WHERE id = ? [2dcb132a]

If this is the intended behaviour do we also need something like a MapUpsertFields modifier which would emulate the MapSetField behaviour for each element in the map?

[crufter]

@mattheath MapSetFields is the right thing to use here, however it seems like it is buggy - it is supposed to atomically update a map (so no need to have MapUpsertFields).

We definitely need more docs.

Edit:

It seems like we have a test for MapSetFields: https://github.com/hailocab/gocassa/blob/master/query_test.go#L343

[mattheath]

supposed to atomically update a map

Do you mean atomically upsert new values into the map, or atomically replace the map with the provided map? The current behaviour is the latter, whereas I am trying to do the former!

[crufter]

I mean atomically upsert new values into the map, as seen in the test:

// Having this initially
c := CustomerWithMap{
        Id: "1",
        Map: map[string]string{
            "3": "Is Odd",
            "6": "Is Even",
        },
}

// Doing a MapSetFields
tbl.Update("1", map[string]interface{}{
        "Map": MapSetFields(map[string]interface{}{
            "2": "Two",
            "4": "Four",
        }),
})

// Will result in this map
CustomerWithMap{
        Id: "1",
        Map: map[string]string{
            "2": "Two",
            "3": "Is Odd",
            "4": "Four",
            "6": "Is Even",
        },
}

[mattheath]

I've just dug into this, and although the test passes, it unfortunately is a bug 😉

In the test we create a struct:

c := CustomerWithMap{
    Id: "1",
    Map: map[string]string{
        "3": "Is Odd",
        "6": "Is Even",
    },
}

This is then Set it into cassandra, which creates the following CQL, and as this uses the CQL Map = ? this replaces any data in the map, with the new map. This is setting, so in my opinion expected behaviour.

UPDATE test_ihopeudonthaveakeyspacenamedlikedthis.customer34213_map_Id SET Map = ? WHERE id = ? [map[3:Is Odd 6:Is Even] 1]

We then Update this, adding the keys 2 and 4 to this using MapSetFields which creates this CQL below. Again, due to the CQL created this replaces the map with the new map, so the keys 3 and 6 no longer exist in cassandra.

UPDATE test_ihopeudonthaveakeyspacenamedlikedthis.customer34213_map_Id SET Map = {'2' : 'Two', '4' : 'Four'}  WHERE id = ? [1]

We also update key 5 using MapSetField("5", "Five!") which creates this CQL below. As this is using the syntax SET Map['5'] = 'Five!' this key:value is merged with the current map contents in cassandra.

UPDATE test_ihopeudonthaveakeyspacenamedlikedthis.customer34213_map_Id SET Map['5'] = 'Five!'  WHERE id = ? [1]

So, at this point the values in cassandra are (from cqlsh):

 id | map
----+-----------------------------------------
  1 | {'2': 'Two', '4': 'Four', '5': 'Five!'}

We then read back the values to assert correctness, and unfortunately this is the point our bug arises. We read back into the original variable c:

if err := tbl.Read("1", &c).Run(); err != nil {
    t.Fatal(err)
}

And as we use use json.Marshal to read the columns back into our *CustomerWithMap (in singleOp.readone here: https://github.com/hailocab/gocassa/blob/5954f4a0ef0e27f2a71490f748c07b50c387fa09/op.go#L61-L65) the json marshaler merges the previous values with the values read from cassandra.

So we pass in a pre-existing CustomerWithMap that already has the keys:

&gocassa.CustomerWithMap{Id:"1", Map:map[string]string{"3":"Is Odd", "6":"Is Even"}}

We then read our map from cassandra which returns:

{"id":"1","map":{"2":"Two","4":"Four","5":"Five!"}}

And json.Unmarshal then merges these keys into the struct, resulting in the expected behaviour!

&gocassa.CustomerWithMap{Id:"1", Map:map[string]string{"4":"Four", "5":"Five!", "3":"Is Odd", "6":"Is Even", "2":"Two"}}

I'm assuming from this that the expected behaviour for MapSetFields is to merge new values with existing values, in which case we need to change the generated CQL to something like the below, which works in Cassandra 2.1.1 or later

UPDATE test_ihopeudonthaveakeyspacenamedlikedthis.customer34213_map_Id SET Map = Map + {'2' : 'Two', '4' : 'Four'}  WHERE id = ? [1]

[crufter]

@mattheath Nice catch!

It seems like all we have to change is a., fix test b., change this line https://github.com/hailocab/gocassa/blob/master/modifiers.go#L113 from

buf.WriteString(fmt.Sprintf("%v = ", name))

to

buf.WriteString(fmt.Sprintf("%v = %v + ", name, name))

So the correct CQL statement will be generated, eg.

instead of

SET Map = {'2' : 'Two', '4' : 'Four'}

this

SET Map = Map + {'2' : 'Two', '4' : 'Four'}

Do you mind creating a PR?

[mattheath]

Yeah, i've tested this and it works fine. Will PR the change and a fixed test 😄

[crufter]

Cheers ;)