V2: Clarify map behavior with message_encoding = DELIMITED #829
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Map fields are syntactic sugar for a repeated message field with field 1 for key and field 2 for value.
In Edition 2023, a file option
features.message_encoding = DELIMITEDdoes not apply to this map entry message, and does not apply to the value field (which can be a message field).This behavior was clarified in protocolbuffers/protobuf#16576 (specifically this line for the map entry and this line for the map value - both would be TYPE_GROUP with DELIMITED).
Our implementation already matched the behavior, this PR just adds additional test coverage.
Our implementation has just one property
delimitedEncodingon map field descriptors that applies to both the map entry and map values. If a future edition changes the behavior, it is possible that we have to provide two distinct properties. IF necessary, we can expose the map entry message for this purpose, which will not be a breaking change.