PDGGK opened a new pull request, #39731:
URL: https://github.com/apache/beam/pull/39731
Fixes #39730.
`AttributeValueCoder.encode` selected the L and M branches with a size
check, so a DynamoDB attribute holding an **empty list** or **empty map** fell
through to the terminal `else` and threw `CoderException("Unknown Type")`.
`hasL()`/`hasM()` are the SDK's way of separating "unset" from "set but
empty", which is exactly the distinction the size check lost.
**The `ss`/`ns`/`bs` guards look identical and are deliberately left
alone.** DynamoDB rejects empty sets, so an empty one there is not a value
worth preserving — relaxing those would be wrong.
**Blast radius.** `MAP_ATTRIBUTE_CODER` re-enters this coder for every
child, so one empty list nested anywhere inside an item makes the whole item
unencodable, not just that attribute. That is the realistic production trigger
and it is what two of the new tests cover.
The decode side already handles both: `case l:` and `case m:` delegate to
`ListCoder`/`MapCoder`, which round-trip empties.
### Compatibility
Wire-compatible **for every value the service can produce**. I want to be
precise rather than overclaim: the change is *not* a no-op for a malformed
`AttributeValue` that sets two types at once — `AttributeValue.builder().l(new
ArrayList<>()).nul(true).build()` currently encodes as `nul` and would now
encode as `l`. An `AttributeValue` carries exactly one type, DynamoDB never
emits such a value, and no ordinary builder usage produces one, but the claim
is "every value the service can produce", not "every possible value".
### Tests
Five cases added to the existing `AttributeValueCoderTest`, which is a plain
in-process encode/decode round trip — no cluster, no AWS account, no localstack.
| case | on master |
|---|---|
| empty `L` round trip | **fails**, `CoderException: Unknown Type` |
| empty `M` round trip | **fails** |
| empty `L` nested in a map | **fails** — shows the parent item is lost |
| empty `M` nested in a list | **fails** |
| `AttributeValue.builder().build()` still rejected | passes **before and
after** — the control |
```
15 tests completed, 4 failed <- on master
BUILD SUCCESSFUL <- with the fix, 15/15
```
The fifth is there so the fix cannot quietly turn "Unknown Type" into
encoding nothing, and because it passes on both versions it also shows the
other four are not trivially red.
Each round trip asserts `hasL()`/`hasM()` on the decoded value in addition
to equality, so a "decoded back as unset" regression would still be caught.
`spotlessCheck`, `checkstyleMain` and `checkstyleTest` pass, as does the
rest of the `dynamodb` package.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
