GH-50508: [C++] Support scalar values in AppendScalars#50584
GH-50508: [C++] Support scalar values in AppendScalars#50584Moemenmohamed24 wants to merge 3 commits into
Conversation
|
|
kou
changed the title
|
Could you use our PR template and fix CI failures? |
Done. I updated the PR description using the project template and applied the required clang-format changes. The latest commit has been pushed, and the CI workflows are currently awaiting maintainer approval. |
|
Approved. FYI: You can check CI results on your fork by enabling GitHub Actions on your fork. |
It seems that the PR description isn't updated yet... |
| ASSERT_OK_AND_ASSIGN(auto v5, MakeScalar(float32(), 3.0f)); | ||
|
|
||
| ScalarVector scalars = {v1, v2, v3, v4, v5}; | ||
|
|
There was a problem hiding this comment.
There's no need to add an empty line after each individual statement, can we compact this a bit and only keep empty lines to distinguish between logically different sequences?
|
|
||
| ASSERT_EQ(builder->length(), 5); | ||
|
|
||
| ASSERT_OK_AND_ASSIGN(auto array, builder->Finish()); |
There was a problem hiding this comment.
Let's also validate the result:
| ASSERT_OK_AND_ASSIGN(auto array, builder->Finish()); | |
| ASSERT_OK_AND_ASSIGN(auto array, builder->Finish()); | |
| ASSERT_OK(array->ValidateFull()); |
| UpdateDimensions(committed_logical_length_, 0); | ||
| return Status::OK(); | ||
| } | ||
|
|
There was a problem hiding this comment.
OTOH, let's please keep an empty line between function and method definitions.
|
I've separately created #50596 to refactor the |
github-actions
Bot
added
awaiting committer review
3 participants
Rationale for this change
RunEndEncodedBuilder::AppendScalaraccepts regular scalar values, such asfloat32directly. However,RunEndEncodedBuilder::AppendScalarsonly handledRunEndEncodedScalarvalues.This made the two APIs inconsistent. This PR updates
AppendScalarsto support regular scalar values consistently withAppendScalar.What changes are included in this PR?
RunEndEncodedBuilder::AppendScalarsto process each scalar throughAppendScalar.RunEndEncodedScalarvalues.Are these changes tested?
Yes. The relevant Run-End Encoded tests pass successfully.
Are there any user-facing changes?
Yes.
RunEndEncodedBuilder::AppendScalarsnow accepts regular scalar values, such asfloat32, consistently withAppendScalar.Breaking changes
No
Critical Fix
No.