Skip to content

Commit

Permalink
Fix the key collision contract for mutable maps (#1040)
Browse files Browse the repository at this point in the history
  • Loading branch information
TristonianJones authored Oct 15, 2024
1 parent b2e4077 commit c936b8b
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 3 deletions.
2 changes: 1 addition & 1 deletion common/types/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ type mutableMap struct {
// Insert implements the traits.MutableMapper interface method, returning true if the key insertion
// succeeds.
func (m *mutableMap) Insert(k, v ref.Val) ref.Val {
if _, found := m.mutableValues[k]; found {
if _, found := m.Find(k); found {
return NewErr("insert failed: key %v already exists", k)
}
m.mutableValues[k] = v
Expand Down
4 changes: 3 additions & 1 deletion ext/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,9 @@ Examples:
Comprehension which converts a map or a list into a map value; however, this
transform expects the entry expression be a map literal. If the transform
produces an entry which duplicates a key in the target map, the comprehension
will error.
will error. Note, that key equality is determined using CEL equality which
asserts that numeric values which are equal, even if they don't have the same
type will cause a key collision.

Elements in the map may optionally be filtered according to a predicate
expression, where elements that satisfy the predicate are transformed.
Expand Down
4 changes: 3 additions & 1 deletion ext/comprehensions.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ const (
//
// Comprehension which converts a map or a list into a map value; however, this transform
// expects the entry expression be a map literal. If the tranform produces an entry which
// duplicates a key in the target map, the comprehension will error.
// duplicates a key in the target map, the comprehension will error. Note, that key
// equality is determined using CEL equality which asserts that numeric values which are
// equal, even if they don't have the same type will cause a key collision.
//
// Elements in the map may optionally be filtered according to a predicate expression, where
// elements that satisfy the predicate are transformed.
Expand Down
4 changes: 4 additions & 0 deletions ext/comprehensions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,10 @@ func TestTwoVarComprehensionsRuntimeErrors(t *testing.T) {
expr: "[1, 1].transformMapEntry(i, v, {v: i})",
err: "insert failed: key 1 already exists",
},
{
expr: `[0, 0u].transformMapEntry(i, v, {v: i})`,
err: "insert failed: key 0 already exists",
},
}
env := testCompreEnv(t)
for i, tst := range tests {
Expand Down

0 comments on commit c936b8b

Please sign in to comment.