DynamoDB expression.NameBuilder.Contains() forces input to string, but interface{} should be supported

[andrewcretin]

Describe the bug

The NameBuilder.Contains() does not allow for filtering sets of types other than strings. For example, a Set of Number []int{1,2,3} can not be filtered using Contains as the value must be cast to a string.

Expected Behavior

I should be able to provide a value of any type, represented by an interface and contains should work so long as the provided value is the same value of the attribute (either primitive or list)

Current Behavior

The contains value is being forced to string (implementation likely done for substring or larger string) but this should also work for searching for a value contained within a list (of any supported type)

Reproduction Steps

See additional info and test case included in possible solution

Possible Solution

In condition.go

// Contains returns a ConditionBuilder representing the result of the
// contains function in DynamoDB Condition Expressions. The resulting
// ConditionBuilder can be used as a part of other Condition Expressions or as
// an argument to the WithCondition() method for the Builder struct.
//
// Example:
//
//	// condition represents the boolean condition of whether the item
//	// attribute "InviteList" has the value "Ben"
//	condition := expression.Name("InviteList").Contains("Ben")
//
//	// condition represents the boolean condition of whether the item
//	// attribute "LocationIds" has the value 2
//	condition := expression.Name("LocationIds").Contains(2)
//
//	// Used in another Condition Expression
//	anotherCondition := expression.Not(condition)
//	// Used to make an Builder
//	builder := expression.NewBuilder().WithCondition(condition)
//
// Expression Equivalent:
//
//	expression.Name("InviteList").Contains("Ben")
//	// Let :ben be an ExpressionAttributeValue representing the value "Ben"
//	"contains (InviteList, :ben)"
func (nb NameBuilder) Contains(val interface{}) ConditionBuilder {
	return Contains(nb, val)
}

and

// Contains returns a ConditionBuilder representing the result of the
// contains function in DynamoDB Condition Expressions. The resulting
// ConditionBuilder can be used as a part of other Condition Expressions or as
// an argument to the WithCondition() method for the Builder struct.
//
// Example:
//
//	// condition represents the boolean condition of whether the item
//	// attribute "InviteList" has the value "Ben"
//	condition := expression.Contains(expression.Name("InviteList"), "Ben")
//
//	// condition represents the boolean condition of whether the item
//	// attribute "LocationIds" has the value 2
//	condition := expression.Contains(expression.Name("LocationIds"), 2)
//
//	// Used in another Condition Expression
//	anotherCondition := expression.Not(condition)
//	// Used to make an Builder
//	builder := expression.NewBuilder().WithCondition(condition)
//
// Expression Equivalent:
//
//	expression.Contains(expression.Name("InviteList"), "Ben")
//	// Let :ben be an ExpressionAttributeValue representing the value "Ben"
//	"contains (InviteList, :ben)"
func Contains(nameBuilder NameBuilder, val interface{}) ConditionBuilder {
	v := ValueBuilder{
		value: val,
	}
	return ConditionBuilder{
		operandList: []OperandBuilder{nameBuilder, v},
		mode:        containsCond,
	}
}

In condition_test.go

func TestContainsCondition(t *testing.T) {
	cases := []struct {
		name         string
		input        ConditionBuilder
		expectedNode exprNode
		err          condErrorMode
	}{
		{
			name:  "basic contains string",
			input: Name("foo").Contains("bar"),
			expectedNode: exprNode{
				children: []exprNode{
					{
						names:   []string{"foo"},
						fmtExpr: "$n",
					},
					{
						values: []types.AttributeValue{
							&types.AttributeValueMemberS{Value: "bar"},
						},
						fmtExpr: "$v",
					},
				},
				fmtExpr: "contains ($c, $c)",
			},
		},
		{
			name:  "basic contains number int",
			input: Name("foo").Contains(2),
			expectedNode: exprNode{
				children: []exprNode{
					{
						names:   []string{"foo"},
						fmtExpr: "$n",
					},
					{
						values: []types.AttributeValue{
							&types.AttributeValueMemberN{Value: "2"},
						},
						fmtExpr: "$v",
					},
				},
				fmtExpr: "contains ($c, $c)",
			},
		},
		{
			name:  "basic contains number float",
			input: Name("foo").Contains(2.23),
			expectedNode: exprNode{
				children: []exprNode{
					{
						names:   []string{"foo"},
						fmtExpr: "$n",
					},
					{
						values: []types.AttributeValue{
							&types.AttributeValueMemberN{Value: "2.23"},
						},
						fmtExpr: "$v",
					},
				},
				fmtExpr: "contains ($c, $c)",
			},
		},
		{
			name:  "contains invalid operand",
			input: Name("").Contains("bar"),
			err:   invalidConditionOperand,
		},
	}

	for _, c := range cases {
		t.Run(c.name, func(t *testing.T) {
			actual, err := c.input.buildTree()
			if c.err != noConditionError {
				if err == nil {
					t.Errorf("expect error %q, got no error", c.err)
				} else {
					if e, a := string(c.err), err.Error(); !strings.Contains(a, e) {
						t.Errorf("expect %q error message to be in %q", e, a)
					}
				}
			} else {
				if err != nil {
					t.Errorf("expect no error, got unexpected Error %q", err)
				}
				if e, a := c.expectedNode, actual; !reflect.DeepEqual(a, e) {
					t.Errorf("expect %v, got %v", e, a)
				}
			}
		})
	}
}

Additional Information/Context

Media showing working implementation on my use case. (local changes to library)

Use Case: Filtering objects that have LocationIds: []int using contains for value 199. Existing code forces string, which fails. When replaced with interface, the expected values are returned

NOT WORKING

broken.mp4

WORKING

fixed.mp4

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]
github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]
github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected] github.com/google/[email protected]
github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected] github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected]
github.com/aws/aws-sdk-go-v2/feature/dynamodb/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]

Compiler and Version used

go version go1.18 darwin/arm64

Operating System and version

Mac OS Sonoma - Version 14.1 Beta (23B5067a)

[andrewcretin]

Local branch changes with all tests passing within expression directory

Screen.Recording.2023-10-18.at.11.34.08.AM.mov

[lucix-aws]

@andrewcretin It's difficult to assess the scope of your changes through this issue and the linked videos. Would you be able to open a PR?

[andrewcretin]

@lucix-aws I certainly can. I would need to be given contributor access first, but can get that done this morning. It's a very minor change

[lucix-aws]

@andrewcretin That's not actually required -- you can fork the repository, commit your changes to a branch in that fork, and then create a PR to merge changes in your branch into our main.

[andrewcretin]

On it 🫡

[andrewcretin]

@lucix-aws PR here: #2324

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.