Use an INSERT rather than an UPSERT when only touching partition/clustering columns #60
Conversation
…tering columns This is protection against a nasty Cassandra bug where UPSERTing a row with all the non-partition/clustering column keys as 'null' causes the entire row to be deleted. This is undesirable behaviour and we should rather use an INSERT to retain the original data https://issues.apache.org/jira/browse/CASSANDRA-11805
suhailpatel
force-pushed
the
partition-validation
branch
from
d940714
to
a14bf22
Compare
| @@ -124,6 +125,25 @@ func transformFields(m map[string]interface{}) { | |||
| } | |||
| } | |||
|
|
|||
| // allFieldValuesAreNullable checks if all the fields passed in contain | |||
| // items that will eventually be marked in Cassandra as null (so we can | |||
| // better predict if we should do an INSERT or an UPSERT). This is to | |||
There was a problem hiding this comment.
| // better predict if we should do an INSERT or an UPSERT). This is to | |
| // better predict if we should do an INSERT or an UPDATE). This is to |
Should this say UPDATE? [CQL Reference]
There was a problem hiding this comment.
I specifically wanted upsert to describe this behaviour. The intention is not to map to a cql command but rather a behaviour of the library (which is to do an upsert)
| for _, value := range fields { | ||
| rv := reflect.ValueOf(value) | ||
| switch rv.Kind() { | ||
| case reflect.Array, reflect.Slice, reflect.Map: |
There was a problem hiding this comment.
Just to check — are these the only types which could be nullable?
I'm trying to see where passing an incorrect type would be filtered out, but I can't see anything that does this? Is it possible for value to be a nil pointer for example?
In Set() below (L177) a map[string]interface{} with a pointer type (say a nil pointer to a string — (*string)(nil)) seems to be passed through and would result in newWriteOp returning an updateOpType?
https://play.golang.org/p/lOA15Bdk5v3
Would this be filtered out later by GoCQL as a field of type *string would be invalid, or converted to type string and therefore nil -> "" ?
There was a problem hiding this comment.
Pointers are filtered out a layer above through reflect.Indirect when the struct is converted into a map to begin with.
Also yes, those are the fields that I found to be susceptible to the nullable behaviour.
There was a problem hiding this comment.
I have added an explicit test for this behaviour just to make sure it's codified
This tests the Execute path to assert queries being generated and making sure we make the right decision between when to use an INSERT vs UPDATE depending on whether we have nullable fields being used
This is protection against a nasty Cassandra bug where UPSERTing a row with all the non-partition/clustering column keys as 'null' causes the entire row to be deleted. This is undesirable behaviour and we should rather use an INSERT to retain the original data