Spec: Clarify variant shredding spec #512
Conversation
aihuaxu
force-pushed
the
clarify-variant-shredding-spec
branch
from
b1ad692 to
28e8dc2
Compare
| The element's `value` field stores the element as Variant-encoded `binary` when the `typed_value` is not present or cannot represent it. | ||
| The `typed_value` field may be omitted when not shredding elements as a specific type. | ||
| When `typed_value` is omitted, `value` must be `required`. | ||
| The `value` field may be omitted when shredding elements as a specific type. |
There was a problem hiding this comment.
It is fine to say that one of the fields must be present and that value can be omitted (though it is not necessary).
There was a problem hiding this comment.
Should we also clarify whether they can/should be optional vs required or either?
There was a problem hiding this comment.
Thanks for reviewing. In line 68, we have the statement
Both
valueandtyped_valueare optional fields used together to encode a single value.
to serve the overall value shredding. I think we don't need to mention in different places.
There was a problem hiding this comment.
That means the test cases which test with the fields being required should also get notes that they are invalid based on the spec?
There was a problem hiding this comment.
I don't think we have such case in the tests. The case we have is: the group should be required but it's passed in as optional. For example:
required group event_type {
optional binary value;
optional binary typed_value (STRING);
}
|
Thanks for the review, @zeroshade! I'll merge this. |
4 participants
Rationale for this change
From the test in apache/arrow-go#455 which validates the cross-language variant implementation, we may need to update a few wording in the spec to make it clear
What changes are included in this PR?
valueandtyped_valuefields can be omitted but not both in array elements.Do these changes have PoC implementations?