dynamodb/expression: name builder for list of lists: foo[10][5] returns: BuildOperand error: invalid parameter: NameBuilder

[advdv]

Describe the bug

As far as I understand it is possible to model a list of lists in dynamodb. So it should be possible to build a name (as part of a condition for example), with something like the code below. Instead, it errors:

package generator_test

import (
	"testing"

	"github.com/aws/aws-sdk-go-v2/feature/dynamodb/expression"
)

func TestNameBuilder(t *testing.T) {
	expr, err := expression.NewBuilder().WithCondition(expression.Name("foo[10][5]").AttributeExists()).Build()
	if err != nil {
		t.Fatalf("failed: %v", err) // BuildOperand error: invalid parameter: NameBuilder
	}
	_ = expr
}

Expected Behavior

It should allow building names for a list of lists

Current Behavior

It returns the following error:

BuildOperand error: invalid parameter: NameBuilder

Reproduction Steps

The code above can be pasted into a _test.go file to get the error.

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue v1.10.19
github.com/aws/aws-sdk-go-v2/feature/dynamodb/expression v1.4.46
github.com/aws/aws-sdk-go-v2/service/dynamodb v1.19.2

Compiler and Version used

go version go1.20.2 darwin/arm64

Operating System and version

MacOS Ventura 13.1

[RanVaknin]

Hi @advanderveer ,

Sorry for the long wait.

I'm able to confirm that nested expressions are not supported. This was probably an oversight with how this feature was implemented.

As a workaround you can pass in the expression directly to the getItemInput:

package main

import (
	"context"
	"github.com/aws/aws-sdk-go-v2/aws"
	"log"

	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/dynamodb"
	"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
)

func main() {
	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithClientLogMode(aws.LogRequestWithBody|aws.LogResponseWithBody))
	if err != nil {
		log.Fatalf("unable to load SDK config, %v", err)
	}

	client := dynamodb.NewFromConfig(cfg)

	params := &dynamodb.GetItemInput{
		TableName:            aws.String("nestedExpressionTable"),
		Key:                  map[string]types.AttributeValue{"PrimaryKey": &types.AttributeValueMemberS{Value: "SomeKey"}},
		ProjectionExpression: aws.String("foo[2][5]"),
	}

	_, err = client.GetItem(context.TODO(), params)
	if err != nil {
		log.Fatalf("failed to get item, %v", err)
	}
}

Will result in a valid response:

{"Item":{"foo":{"L":[{"L":[{"N":"96"}]}]}}}

I'll discuss this with the team for further investigation.

Thanks,
Ran~

[badu]

Hello everybody,

after investigating, the smallest change would be to have a new NameBuilder constructor (because names field is private) that accepts a slice of strings, as following:

func Names(names []string) NameBuilder {
	return NameBuilder{ names: names }
}

This could be used by developers having a slice producer function, as the example below:

func BuildNestedNames(source string) []string {
	if len(source) == 0 {
		return nil
	}

	re := regexp.MustCompile(`((\w+)(\[(\w+)\])?(\.(\w+)(\[(\w+)\])?)*)`)

	var names []string
	parts := strings.Split(source, ".")

	for _, part := range parts {
		matches := re.FindAllStringSubmatch(part, -1)

		if matches == nil {
			names = append(names, part)
			continue
		}

		for _, match := range matches {
			if len(match) == 0 {
				continue
			}

			fullMatch := match[1]
			subMatches := strings.Split(fullMatch, ".")
			for _, subMatch := range subMatches {
				bracketMatch := regexp.MustCompile(`(\w+)\[(\w+)\]`).FindStringSubmatch(subMatch)

				if bracketMatch == nil || len(bracketMatch) != 3 {
					names = append(names, subMatch)
					continue
				}

				names = append(names, bracketMatch[1], bracketMatch[2])
			}
		}

	}

	return names
}

The result of the slice producer function would be passed to the constructor, permitting the developer to customize according to needs.

In the example above, the RegExp is doing:

• Top-level name: (\w+) captures the initial attribute name (e.g., "foo", "bar", "nested").
• Optional bracket notation: ([(\w+)])? captures the optional bracket and index (e.g., "[10]", "[5]"). The ? makes it optional.
• Repeating dot and bracket notation: (.(\w+)([(\w+)])?)* allows for multiple levels of nesting, separated by dots. The * allows for zero or more occurrences.

Additionally or alternatively, we can offer a setter for the NameBuilder names field.

Waiting for feedback before opening a PR.

[lucix-aws]

This seems more there's like a lack of functionality in the existing builder behavior, rather than something where we need new APIs.

Notice that expressions of semi-arbitrary complexity already do function:

package main

import "github.com/aws/aws-sdk-go-v2/feature/dynamodb/expression"

func main() {
	_, err := expression.NewBuilder().
		WithCondition(
			expression.Name("foo[0].bar").AttributeExists(),
		).Build()
	if err != nil {
		panic(err)
	}
}

So, at a high level, something like the following should also function, but it currently does not, which is what needs to be addressed here:

package main

import "github.com/aws/aws-sdk-go-v2/feature/dynamodb/expression"

func main() {
	_, err := expression.NewBuilder().
		WithCondition(
			// double-index
			expression.Name("foo[0][1].bar").AttributeExists(),
		).Build()
	if err != nil {
		panic(err)
	}
}

[lucix-aws]

Adding a direct Names([]string) API doesn't appear to solve this problem as far as I can tell, or at least, it's producing fundamentally different output, even in the case for what's currently supported. Plugging in your proposed code above, I get two different results when attempting to build foo[0]:

	expr, err := expression.NewBuilder().
		WithCondition(expression.Name("foo[0]").AttributeExists()).Build()
	if err != nil {
		panic(err)
	}

	fmt.Println(expr.Names())
	fmt.Println(*expr.Condition())

produces

map[#0:foo]
attribute_exists (#0[0])

whereas WithCondition(expression.Names(BuildNestedNames("foo[0]")).AttributeExists()) produces

map[#0:foo #1:0]
attribute_exists (#0.#1)

i.e. a field index, rather than what we wanted which was an array index.

(and note that something like the described use case of "foo[0][1]" then produces

map[#0:foo #1:0 #2:1]
attribute_exists (#0.#1.#2)