Skip to content

Commit

Permalink
CASSGO-42: don't panic if no applied column is returned
Browse files Browse the repository at this point in the history
I also added checks against MapScan failing which could also trigger this bug.

Several integration tests were added to validate various edge cases.

Patch by James Hartig for CASSGO-42
  • Loading branch information
jameshartig committed Dec 27, 2024
1 parent 37030fb commit 97336ec
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Retry policy now takes into account query idempotency (CASSGO-27)
- Don't return error to caller with RetryType Ignore (CASSGO-28)
- Don't panic in MapExecuteBatchCAS if no `[applied]` column is returned (CASSGO-42)

## [1.7.0] - 2024-09-23

Expand Down
56 changes: 53 additions & 3 deletions cassandra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"context"
"errors"
"fmt"
"github.com/stretchr/testify/require"
"io"
"math"
"math/big"
Expand All @@ -45,6 +44,8 @@ import (
"time"
"unicode"

"github.com/stretchr/testify/require"

"gopkg.in/inf.v0"
)

Expand Down Expand Up @@ -476,7 +477,7 @@ func TestCAS(t *testing.T) {
if applied, _, err := session.ExecuteBatchCAS(successBatch, &titleCAS, &revidCAS, &modifiedCAS); err != nil {
t.Fatal("insert:", err)
} else if applied {
t.Fatalf("insert should have been applied: title=%v revID=%v modified=%v", titleCAS, revidCAS, modifiedCAS)
t.Fatalf("insert should have not been applied: title=%v revID=%v modified=%v", titleCAS, revidCAS, modifiedCAS)
}

insertBatch := session.Batch(LoggedBatch)
Expand All @@ -492,7 +493,7 @@ func TestCAS(t *testing.T) {
if applied, iter, err := session.ExecuteBatchCAS(failBatch, &titleCAS, &revidCAS, &modifiedCAS); err != nil {
t.Fatal("insert:", err)
} else if applied {
t.Fatalf("insert should have been applied: title=%v revID=%v modified=%v", titleCAS, revidCAS, modifiedCAS)
t.Fatalf("insert should have not been applied: title=%v revID=%v modified=%v", titleCAS, revidCAS, modifiedCAS)
} else {
if scan := iter.Scan(&applied, &titleCAS, &revidCAS, &modifiedCAS); scan && applied {
t.Fatalf("insert should have been applied: title=%v revID=%v modified=%v", titleCAS, revidCAS, modifiedCAS)
Expand All @@ -503,6 +504,55 @@ func TestCAS(t *testing.T) {
t.Fatal("scan:", err)
}
}

casMap = make(map[string]interface{})
if applied, err := session.Query(`SELECT revid FROM cas_table WHERE title = ?`,
title+"_foo").MapScanCAS(casMap); err != nil {
t.Fatal("select:", err)
} else if applied {
t.Fatal("select shouldn't have returned applied")
}

if _, err := session.Query(`SELECT revid FROM cas_table WHERE title = ?`,
title+"_foo").ScanCAS(&revidCAS); err == nil {
t.Fatal("select: should have returned an error")
}

notCASBatch := session.Batch(LoggedBatch)
notCASBatch.Query("INSERT INTO cas_table (title, revid, last_modified) VALUES (?, ?, ?)", title+"_baz", revid, modified)
casMap = make(map[string]interface{})
if _, _, err := session.MapExecuteBatchCAS(notCASBatch, casMap); err != ErrNotFound {
t.Fatal("insert should have returned not found:", err)
}

notCASBatch = session.Batch(LoggedBatch)
notCASBatch.Query("INSERT INTO cas_table (title, revid, last_modified) VALUES (?, ?, ?)", title+"_baz", revid, modified)
casMap = make(map[string]interface{})
if _, _, err := session.ExecuteBatchCAS(notCASBatch, &revidCAS); err != ErrNotFound {
t.Fatal("insert should have returned not found:", err)
}

failBatch = session.Batch(LoggedBatch)
failBatch.Query("UPDATE cas_table SET last_modified = DATEOF(NOW()) WHERE title='_foo' AND revid=3e4ad2f1-73a4-11e5-9381-29463d90c3f0 IF last_modified = ?", modified)
if _, _, err := session.ExecuteBatchCAS(failBatch, new(bool)); err == nil {
t.Fatal("update should have errored")
}
// make sure MapScanCAS does not panic when MapScan fails
casMap = make(map[string]interface{})
casMap["last_modified"] = false
if _, err := session.Query(`UPDATE cas_table SET last_modified = DATEOF(NOW()) WHERE title='_foo' AND revid=3e4ad2f1-73a4-11e5-9381-29463d90c3f0 IF last_modified = ?`,
modified).MapScanCAS(casMap); err == nil {
t.Fatal("update should hvae errored", err)
}

// make sure MapExecuteBatchCAS does not panic when MapScan fails
failBatch = session.Batch(LoggedBatch)
failBatch.Query("UPDATE cas_table SET last_modified = DATEOF(NOW()) WHERE title='_foo' AND revid=3e4ad2f1-73a4-11e5-9381-29463d90c3f0 IF last_modified = ?", modified)
casMap = make(map[string]interface{})
casMap["last_modified"] = false
if _, _, err := session.MapExecuteBatchCAS(failBatch, casMap); err == nil {
t.Fatal("update should have errored")
}
}

func TestDurationType(t *testing.T) {
Expand Down
19 changes: 15 additions & 4 deletions helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ func TupleColumnName(c string, n int) string {
return fmt.Sprintf("%s[%d]", c, n)
}

// RowData returns the RowData for the iterator.
func (iter *Iter) RowData() (RowData, error) {
if iter.err != nil {
return RowData{}, iter.err
Expand All @@ -334,6 +335,7 @@ func (iter *Iter) RowData() (RowData, error) {
if c, ok := column.TypeInfo.(TupleTypeInfo); !ok {
val, err := column.TypeInfo.NewWithError()
if err != nil {
iter.err = err
return RowData{}, err
}
columns = append(columns, column.Name)
Expand All @@ -343,6 +345,7 @@ func (iter *Iter) RowData() (RowData, error) {
columns = append(columns, TupleColumnName(column.Name, i))
val, err := elem.NewWithError()
if err != nil {
iter.err = err
return RowData{}, err
}
values = append(values, val)
Expand All @@ -364,7 +367,10 @@ func (iter *Iter) rowMap() (map[string]interface{}, error) {
return nil, iter.err
}

rowData, _ := iter.RowData()
rowData, err := iter.RowData()
if err != nil {
return nil, err
}
iter.Scan(rowData.Values...)
m := make(map[string]interface{}, len(rowData.Columns))
rowData.rowMap(m)
Expand All @@ -379,7 +385,10 @@ func (iter *Iter) SliceMap() ([]map[string]interface{}, error) {
}

// Not checking for the error because we just did
rowData, _ := iter.RowData()
rowData, err := iter.RowData()
if err != nil {
return nil, err
}
dataToReturn := make([]map[string]interface{}, 0)
for iter.Scan(rowData.Values...) {
m := make(map[string]interface{}, len(rowData.Columns))
Expand Down Expand Up @@ -435,8 +444,10 @@ func (iter *Iter) MapScan(m map[string]interface{}) bool {
return false
}

// Not checking for the error because we just did
rowData, _ := iter.RowData()
rowData, err := iter.RowData()
if err != nil {
return false
}

for i, col := range rowData.Columns {
if dest, ok := m[col]; ok {
Expand Down
22 changes: 17 additions & 5 deletions session.go
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ func (s *Session) ExecuteBatchCAS(batch *Batch, dest ...interface{}) (applied bo
iter.Scan(&applied)
}

return applied, iter, nil
return applied, iter, iter.err
}

// MapExecuteBatchCAS executes a batch operation much like ExecuteBatchCAS,
Expand All @@ -798,8 +798,14 @@ func (s *Session) MapExecuteBatchCAS(batch *Batch, dest map[string]interface{})
return false, nil, err
}
iter.MapScan(dest)
applied = dest["[applied]"].(bool)
delete(dest, "[applied]")
if iter.err != nil {
return false, iter, iter.err
}
// check if [applied] was returned, otherwise it might not be CAS
if _, ok := dest["[applied]"]; ok {
applied = dest["[applied]"].(bool)
delete(dest, "[applied]")
}

// we usually close here, but instead of closing, just returin an error
// if MapScan failed. Although Close just returns err, using Close
Expand Down Expand Up @@ -1387,8 +1393,14 @@ func (q *Query) MapScanCAS(dest map[string]interface{}) (applied bool, err error
return false, err
}
iter.MapScan(dest)
applied = dest["[applied]"].(bool)
delete(dest, "[applied]")
if iter.err != nil {
return false, iter.err
}
// check if [applied] was returned, otherwise it might not be CAS
if _, ok := dest["[applied]"]; ok {
applied = dest["[applied]"].(bool)
delete(dest, "[applied]")
}

return applied, iter.Close()
}
Expand Down

0 comments on commit 97336ec

Please sign in to comment.