Skip to content

service/dynamodb/expression: Allow AttributeValue as a value to BuildOperand. #3057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 20, 2020

Conversation

bullgare
Copy link
Contributor

@bullgare bullgare commented Dec 27, 2019

Now you have no way to specify a dynamodb.AttributeValue as a parameter to expression.BuildOperand method.
If you do so, it will be treated as any other struct, and instead of just passing a value, it will Marshal it's inner struct (all the {...BOOL: true, NULL: true, ...} fields that are needed for inner implementation).

That is a problem when you (for example) have a struct that was already been Marshalled:

package main

import (
	"fmt"

	"github.com/aws/aws-sdk-go/service/dynamodb"
	"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbattribute"
	"github.com/aws/aws-sdk-go/service/dynamodb/expression"
)

type T struct {
	ID    uint64  `dynamodbav:"id"`
	Code  string  `dynamodbav:"code"`
	Total float32 `dynamodbav:"amount"`
}

func main() {
	t := T{
		ID:    1,
		Code:  "code",
		Total: 10.5,
		// it can have dozens of fields
	}

	// it is now Marshalled into proper structs
	av, err := dynamodbattribute.MarshalMap(t)
	if err != nil {
		panic(err)
	}

	updateExpr := buildUpdateExpressionFrom(av)

	builder := expression.NewBuilder().
		WithUpdate(updateExpr)

	expr, err := builder.Build()
	if err != nil {
		panic(err)
	}

	fmt.Printf("%q", expr) // it will have a mess now. fixed in this PR
}

func buildUpdateExpressionFrom(av map[string]*dynamodb.AttributeValue) expression.UpdateBuilder {
	var expr expression.UpdateBuilder

	for attr, value := range av {
		expr = expr.Set(expression.Name(attr), expression.Value(value)) // <--- here is a problem now
	}

	return expr
}

@bullgare
Copy link
Contributor Author

It can not be done on consumer side as expression.OperandBuilder interface expects BuildOperand method which works with exact implementation of expression.Operand which has unexportalbe fields.

@jasdel jasdel added the needs-review This issue or pull request needs review from a core team member. label Jan 2, 2020
@jasdel jasdel changed the title Dynamodb expression. Allow AttributeValue as a value to BuildOperand. service/dynamodb/expression: Allow AttributeValue as a value to BuildOperand. Jan 20, 2020
@jasdel jasdel merged commit 87f780f into aws:master Jan 20, 2020
aws-sdk-go-automation pushed a commit that referenced this pull request Jan 20, 2020
===

### Service Client Updates
* `service/alexaforbusiness`: Updates service API and documentation
* `service/application-insights`: Updates service API, documentation, and paginators
* `service/ec2`: Updates service API, documentation, and paginators
  * This release provides support for a preview of bringing your own IPv6 addresses (BYOIP for IPv6) for use in AWS.
* `service/kms`: Updates service API and documentation
  * The ConnectCustomKeyStore operation now provides new error codes (USER_LOGGED_IN and USER_NOT_FOUND) for customers to better troubleshoot if their connect custom key store operation fails. Password length validation during CreateCustomKeyStore now also occurs on the client side.
* `service/lambda`: Updates service API and documentation
  * Added reason codes to StateReasonCode (InvalidSubnet, InvalidSecurityGroup) and LastUpdateStatusReasonCode (SubnetOutOfIPAddresses, InvalidSubnet, InvalidSecurityGroup) for functions that connect to a VPC.
* `service/monitoring`: Updates service API and documentation
  * Updating DescribeAnomalyDetectors API to return AnomalyDetector Status value in response.

### SDK Bugs
* `service/dynamodb/expression`: Allow AttributeValue as a value to BuildOperand. ([#3057](#3057))
  * This change fixes the SDK's behavior with DynamoDB Expression builder to not double marshal AttributeValues when used as BuildOperands, `Value` type. The AttributeValue will be used in the expression as the specific value set in the AttributeValue, instead of encoded as another AttributeValue.
aws-sdk-go-automation added a commit that referenced this pull request Jan 20, 2020
Release v1.28.6 (2020-01-20)
===

### Service Client Updates
* `service/alexaforbusiness`: Updates service API and documentation
* `service/application-insights`: Updates service API, documentation, and paginators
* `service/ec2`: Updates service API, documentation, and paginators
  * This release provides support for a preview of bringing your own IPv6 addresses (BYOIP for IPv6) for use in AWS.
* `service/kms`: Updates service API and documentation
  * The ConnectCustomKeyStore operation now provides new error codes (USER_LOGGED_IN and USER_NOT_FOUND) for customers to better troubleshoot if their connect custom key store operation fails. Password length validation during CreateCustomKeyStore now also occurs on the client side.
* `service/lambda`: Updates service API and documentation
  * Added reason codes to StateReasonCode (InvalidSubnet, InvalidSecurityGroup) and LastUpdateStatusReasonCode (SubnetOutOfIPAddresses, InvalidSubnet, InvalidSecurityGroup) for functions that connect to a VPC.
* `service/monitoring`: Updates service API and documentation
  * Updating DescribeAnomalyDetectors API to return AnomalyDetector Status value in response.

### SDK Bugs
* `service/dynamodb/expression`: Allow AttributeValue as a value to BuildOperand. ([#3057](#3057))
  * This change fixes the SDK's behavior with DynamoDB Expression builder to not double marshal AttributeValues when used as BuildOperands, `Value` type. The AttributeValue will be used in the expression as the specific value set in the AttributeValue, instead of encoded as another AttributeValue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review This issue or pull request needs review from a core team member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants