robertwb commented on code in PR #31092:
URL: https://github.com/apache/beam/pull/31092#discussion_r1586618153
##########
model/fn-execution/src/main/proto/org/apache/beam/model/fn_execution/v1/beam_fn_api.proto:
##########
@@ -1063,6 +1087,10 @@ message StateAppendRequest {
// Represents a part of a logical byte stream. Elements within
// the logical byte stream are encoded in the nested context and
// multiple append requests are concatenated together.
+ // For OrderedListState, elements of TimeStampedValue<T> should be encoded
+ // with TimestampedValueCoder.of(LengthPrefixCoder.of(Coder<T>)), so that
+ // the request handler knows how to decode timestamps from the data without
Review Comment:
Given these are sort _keys_, let's keep them in the key slot. (In other
words, I'd rather optimized for cleanliness of the spec over matching an
existing internal implementation detail.)
It does not matter that the `TimestampedValue$TimestampedValueCoder` does
not agree here, we would just use a different `Coder<TimestampedValue>`. (We
need to explicitly ensure that the coder for T is LP anyway, so we shouldn't be
inferring coders here.)
--
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: github-unsubscr...@beam.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
