enhance: support nullable group by keys #41313
Conversation
Signed-off-by: Ted Xu <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tedxu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
mergify
bot
added
dco-passed
|
@tedxu go-sdk check failed, comment |
|
@tedxu E2e jenkins job failed, comment |
|
@tedxu cpp-unit-test check failed, comment |
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (58.18%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #41313 +/- ##
==========================================
- Coverage 80.58% 80.57% -0.01%
==========================================
Files 1476 1476
Lines 209861 209902 +41
==========================================
+ Hits 169111 169137 +26
- Misses 34615 34635 +20
+ Partials 6135 6130 -5
🚀 New features to boost your workflow:
|
Signed-off-by: Ted Xu <[email protected]>
|
@tedxu E2e jenkins job failed, comment |
|
/run-cpu-e2e |
|
@tedxu E2e jenkins job failed, comment |
|
/run-cpu-e2e |
| vector_iterators[vec_iter_idx++]->AddIterator(kw_iterator); | ||
| } | ||
| for (auto vector_iter : vector_iterators) { | ||
| for (const auto& vector_iter : vector_iterators) { | ||
| vector_iter->seal(); |
There was a problem hiding this comment.
good refine for bad coding habit
| std::string val = | ||
| std::get<std::string>(group_by_vals[idx].value()); | ||
| *(field_data->mutable_data()->Add()) = val; | ||
| } else { |
There was a problem hiding this comment.
std::getstd::string(group_by_vals[idx]) will return a string&
so in this way, one copy constructor and one copy assignment operator is called
perf better:
*(field_data->mutable_data()->Add()) = std::move(std::getstd::string(group_by_vals[idx].value()));
only one move assignment operator is called
|
/lgtm |
See #36264