[ObjC] Skip 0-length slices when writing to CFWriteStream (#37246)#37255
[ObjC] Skip 0-length slices when writing to CFWriteStream (#37246)#37255ziglerari wants to merge 1 commit intogrpc:masterfrom
Conversation
|
|
| for (size_t i = 0; i < data->Count(); i++) { | ||
| auto slice = data->RefSlice(i); | ||
| if (slice.size() == 0) { | ||
| continue; |
There was a problem hiding this comment.
Sanity check is failing because of formatting, can you update accrodingly?
| continue; | ||
| } | ||
|
|
||
| size_t written_size = |
There was a problem hiding this comment.
As you mentioned in the issue, we should not use unsigned here but do error handling if the return value is -1.
I guess we can have a follow up PR to fix it.
There was a problem hiding this comment.
@HannahShiSFB Could you recommend a fix here if it's very simple? If not we can always do a follow-up fix.
There was a problem hiding this comment.
|
Thanks for the fix, BTW do you know in what cases that Arrow Flight needs to send a zero bytes buffer? |
@HannahShiSFB Arrow, in certain scenarios, utilizes a zero-byte buffer in conjunction with its Array structure. This is specifically related to the validity bitmap buffer, which is used to index valid entries (i.e., entries that are not null) within an Array. According to the Arrow specification, if all entries in an Array are valid, implementations may opt to indicate this by using a null buffer, effectively signaling that all entries are valid without explicitly storing this information. This approach is described as a "lazy" signaling mechanism for full validity (refer to Arrow Spec). |
1d828d2 to
21c9db1
Compare
sampajano
left a comment
There was a problem hiding this comment.
Thanks for the change and detailed explanations! 😃
|
@HannahShiSFB what prevents us from merging this PR? is there anything from my side? |
|
@ziglerari Hi.. I think the PR is not being merged due to Python failure, which should be unrelated to this PR.. Let me re-trigger CI to see if it clears :) |
|
UPDATE: Tests have cleared. Will merge soon! :) |
grpc#37255) grpc#37246 Closes grpc#37255 COPYBARA_INTEGRATE_REVIEW=grpc#37255 from ziglerari:cfstream-zero-length-buffers 21c9db1 PiperOrigin-RevId: 657770743
follow up of grpc#37255 Closes grpc#37344 COPYBARA_INTEGRATE_REVIEW=grpc#37344 from HannahShiSFB:cfstream-write-error-handling c9a751d PiperOrigin-RevId: 663329438
#37246